<<< Date Index >>>     <<< Thread Index >>>

#2846: Security vulnerability in APOP authentication



On Sunday, 18 March 2007 at 17:36, Rocco Rutte wrote:
>  I was looking at some mutt code for this issue and other issues that 
>  report broken threading upon invalid message-ids. It seems that mutt 
>  happily accepts the following syntax: '<.*>' which is just plain wrong.
> 
>  I looked at rfc822.c to try to reuse address parsing for parsing 
>  message-ids but failed since I didn't have much time and the quote is 
>  quite complex.
> 
>  Even though adopting your code for mutt would be quite easy, I'm not 
>  yet sure what to do in case of validation errors.
> 
>  Say we get '<foobar>' during APOP authentication; should be really 
>  reject that and report failed authentication? If a message has 
>  '<foobar>' as message-id and others have it in their References: 
>  header, should we really ignore it and break threading?

Here's a patch that does a really minimal check that the message ID is
of the form <x@y> where x and y are between ASCII 0 and 127. I hope
that it's enough to thwart the MD5 collision attack, but liberal
enough to tolerate the range of broken POP servers out there. The @y
test could be easily removed if necessary.

Comments?
# HG changeset patch
# User Brendan Cully <brendan@xxxxxxxxxx>
# Date 1175552458 25200
# Node ID aad620cbe1bfc79a933a5f3dcf466b141f890ed6
# Parent  eabef30c9344f388dffcf4e4d9d6637229f6957c
Validate msgid in APOP authentication. Closes #2846

diff -r eabef30c9344 -r aad620cbe1bf pop_auth.c
--- a/pop_auth.c        Mon Apr 02 14:33:29 2007 -0700
+++ b/pop_auth.c        Mon Apr 02 15:20:58 2007 -0700
@@ -183,6 +183,13 @@ static pop_auth_res_t pop_auth_apop (POP
   if (!pop_data->timestamp)
     return POP_A_UNAVAIL;
 
+  if (rfc822_valid_msgid (pop_data->timestamp) < 0)
+  {
+    mutt_error _("POP timestamp is invalid!");
+    mutt_sleep (2);
+    return POP_A_UNAVAIL;
+  }
+
   mutt_message _("Authenticating (APOP)...");
 
   /* Compute the authentication hash to send to the server */
diff -r eabef30c9344 -r aad620cbe1bf rfc822.c
--- a/rfc822.c  Mon Apr 02 14:33:29 2007 -0700
+++ b/rfc822.c  Mon Apr 02 15:20:58 2007 -0700
@@ -792,6 +792,52 @@ ADDRESS *rfc822_append (ADDRESS **a, ADD
   return tmp;
 }
 
+/* incomplete. Only used to thwart the APOP MD5 attack (#2846). */
+int rfc822_valid_msgid (const char *msgid)
+{
+  /* msg-id         = "<" addr-spec ">"
+   * addr-spec      = local-part "@" domain
+   * local-part     = word *("." word)
+   * word           = atom / quoted-string
+   * atom           = 1*<any CHAR except specials, SPACE and CTLs>
+   * CHAR           = ( 0.-127. )
+   * specials       = "(" / ")" / "<" / ">" / "@"
+                    / "," / ";" / ":" / "\" / <">
+                   / "." / "[" / "]"
+   * SPACE          = ( 32. )
+   * CTLS           = ( 0.-31., 127.)
+   * quoted-string  = <"> *(qtext/quoted-pair) <">
+   * qtext          = <any CHAR except <">, "\" and CR>
+   * CR             = ( 13. )
+   * quoted-pair    = "\" CHAR
+   * domain         = sub-domain *("." sub-domain)
+   * sub-domain     = domain-ref / domain-literal
+   * domain-ref     = atom
+   * domain-literal = "[" *(dtext / quoted-pair) "]"
+   */
+
+  char* dom;
+  unsigned int l, i;
+
+  if (!msgid || !*msgid)
+    return -1;
+
+  l = mutt_strlen (msgid);
+  if (l < 5) /* <atom@atom> */
+    return -1;
+  if (msgid[0] != '<' || msgid[l-1] != '>')
+    return -1;
+  if (!(dom = strrchr (msgid, '@')))
+    return -1;
+
+  /* TODO: complete parser */
+  for (i = 0; i < l; i++)
+    if (msgid[i] > 127)
+      return -1;
+
+  return 0;
+}
+
 #ifdef TESTING
 int safe_free (void **p)       /* __SAFE_FREE_CHECKED__ */
 {
diff -r eabef30c9344 -r aad620cbe1bf rfc822.h
--- a/rfc822.h  Mon Apr 02 14:33:29 2007 -0700
+++ b/rfc822.h  Mon Apr 02 15:20:58 2007 -0700
@@ -52,6 +52,7 @@ void rfc822_write_address_single (char *
 void rfc822_write_address_single (char *, size_t, ADDRESS *, int);
 void rfc822_free_address (ADDRESS **addr);
 void rfc822_cat (char *, size_t, const char *, const char *);
+int rfc822_valid_msgid (const char *msgid);
 
 extern int RFC822Error;
 extern const char *RFC822Errors[];