From b9eb1a366ff0127c49a0589905435680b137d49f Mon Sep 17 00:00:00 2001
From: Mark Wooding <mdw@distorted.org.uk>
Date: Sun, 23 Jun 2013 23:24:17 +0100
Subject: [PATCH] Fix limits on reading user policy files.

The old code only checked the line code after a successful policy line.
A policy file consisting only of blank lines, comments, and erroneous
lines could cause the daemon to loop forever.

Modify `read_policy_file' to return `T_*' codes (like the comment says
it should!), and handle the various codes in the caller.  Most
particularly, `client_line' gives up after the first error to avoid
spamming the log.  Getting this to work properly involves applying a
different ordering to the `T_*' codes.
---
 policy.c | 19 ++++++++++---------
 yaid.c   | 13 ++++++++-----
 yaid.h   |  4 ++--
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/policy.c b/policy.c
index 57ce979..9a9e2e2 100644
--- a/policy.c
+++ b/policy.c
@@ -492,22 +492,20 @@ int read_policy_file(struct policy_file *pf)
     t = parse_policy(pf->fp, &pf->p);
     switch (t) {
       case T_OK:
+      case T_EOL:
 	nextline(pf->fp);
-	return (0);
+	return (t);
       case T_ERROR:
 	logmsg(pf->q, LOG_ERR, "%s:%d: parse error in %s",
 	       pf->name, pf->lno, pf->what);
 	pf->err = 1;
-	break;
+	return (t);
       case T_EOF:
 	if (ferror(pf->fp)) {
 	  logmsg(pf->q, LOG_ERR, "failed to read %s `%s': %s",
 		 pf->what, pf->name, strerror(errno));
 	}
-	return (-1);
-      case T_EOL:
-	nextline(pf->fp);
-	break;
+	return (t);
       default:
 	abort();
     }
@@ -530,12 +528,15 @@ int load_policy_file(const char *file, policy_v *pv)
 {
   struct policy_file pf;
   policy_v v = DA_INIT;
+  int t = 0;
 
   if (open_policy_file(&pf, file, "policy file", 0, 0))
     return (-1);
-  while (!read_policy_file(&pf)) {
-    DA_PUSH(&v, pf.p);
-    init_policy(&pf.p);
+  while ((t = read_policy_file(&pf)) < T_EOF) {
+    if (t == T_OK) {
+      DA_PUSH(&v, pf.p);
+      init_policy(&pf.p);
+    }
   }
   close_policy_file(&pf);
   if (!pf.err) {
diff --git a/yaid.c b/yaid.c
index 1dfa947..232a779 100644
--- a/yaid.c
+++ b/yaid.c
@@ -629,7 +629,7 @@ static void client_line(char *line, size_t len, void *p)
   struct policy upol = POLICY_INIT(A_LIMIT);
   struct policy_file pf;
   char buf[16];
-  int i;
+  int i, t;
 
   /* If the connection has closed, then tidy stuff away. */
   c->q.s[L].port = c->q.s[R].port = 0;
@@ -707,11 +707,11 @@ static void client_line(char *line, size_t len, void *p)
     dstr_putf(&d, "%s/.yaid.policy", pw->pw_dir);
     if (open_policy_file(&pf, d.buf, "user policy file", &c->q, OPF_NOENTOK))
       continue;
-    while (!read_policy_file(&pf)) {
+    while ((t = read_policy_file(&pf)) < T_ERROR) {
 
-      /* Give up after 100 lines.  If the user's policy is that complicated,
-       * something's gone very wrong.  Or there's too much commentary or
-       * something.
+      /* Give up after 100 lines or if there's an error.  If the user's
+       * policy is that complicated, something's gone very wrong.  Or there's
+       * too much commentary or something.
        */
       if (pf.lno > 100) {
 	logmsg(&c->q, LOG_ERR, "%s:%d: user policy file too long",
@@ -719,6 +719,9 @@ static void client_line(char *line, size_t len, void *p)
 	break;
       }
 
+      /* If this was a blank line, just go around again. */
+      if (t != T_OK) continue;
+
       /* If this isn't a match, go around for the next rule. */
       if (!match_policy(&pf.p, &c->q)) continue;
 
diff --git a/yaid.h b/yaid.h
index ef3e726..a91ea27 100644
--- a/yaid.h
+++ b/yaid.h
@@ -330,8 +330,8 @@ extern int match_policy(const struct policy */*p*/,
 enum {
   T_OK,					/* Successful: results returned */
   T_EOL,				/* End-of-line found immediately */
-  T_EOF,				/* End-of-file found immediately */
-  T_ERROR				/* Some kind of error occurred */
+  T_ERROR,				/* Some kind of error occurred */
+  T_EOF					/* End-of-file found immediately */
 };
 
 /* A context for parsing a policy file. */
-- 
2.11.0