From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Thu, 28 Dec 2017 20:51:28 +0000 (+0000)
Subject: DKIM: tighter checking while parsing signature headers.  Bug 2217
X-Git-Tag: exim-4_91_RC1~131
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/a05d3e341fae03426a9caa9c5e9ce8fe60e3d384

DKIM: tighter checking while parsing signature headers.  Bug 2217
---

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index d13004b8c..7ec669b1c 100644
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -19,6 +19,12 @@ JH/02 Disallow '/' characters in queue names specified for the "queue=" ACL
 JH/03 Fix pgsql lookup for multiple result-tuples with a single column.
       Previously only the last row was returned.
 
+JH/04 Bug 2217: Tighten up the parsing of DKIM signature headers. Previously
+      we assumed that tags in the header were well-formed, and parsed the
+      element content after inspecting only the first char of the tag.
+      Assumptions at that stage could crash the receive process on malformed
+      input.
+
 
 Exim version 4.90
 -----------------
diff --git a/src/src/pdkim/pdkim.c b/src/src/pdkim/pdkim.c
index 20366a461..b884671da 100644
--- a/src/src/pdkim/pdkim.c
+++ b/src/src/pdkim/pdkim.c
@@ -490,7 +490,12 @@ for (p = raw_hdr; ; p++)
 
     if (c == ';' || c == '\0')
       {
-      if (cur_tag && cur_val)
+      /* We must have both tag and value, and tags must be one char except
+      for the possibility of "bh". */
+
+      if (  cur_tag && cur_val
+	 && (cur_tag->ptr == 1 || *cur_tag->s == 'b')
+	 )
         {
 	(void) string_from_gstring(cur_val);
 	pdkim_strtrim(cur_val);
@@ -500,8 +505,14 @@ for (p = raw_hdr; ; p++)
 	switch (*cur_tag->s)
 	  {
 	  case 'b':
-	    pdkim_decode_base64(cur_val->s,
-			    cur_tag->s[1] == 'h' ? &sig->bodyhash : &sig->sighash);
+	    switch (cur_tag->s[1])
+	      {
+	      case '\0': pdkim_decode_base64(cur_val->s, &sig->sighash); break;
+	      case 'h':  if (cur_tag->ptr == 2)
+			   pdkim_decode_base64(cur_val->s, &sig->bodyhash);
+			 break;
+	      default:   break;
+	      }
 	    break;
 	  case 'v':
 	      /* We only support version 1, and that is currently the