Re: [Nmh-workers] nmh 1.2 failed in doing smtp authentication

[Peter Maydell]

Ken Hornstein wrote:
>(I just saw your patch ... sasl_decode64() can return SASL_CONTINUE?
>Huh, I guess it can.  But it looks like technically you need to handle
>that on the client in some special way, but I don't see what you're
>supposed to do.  If this solves the problem, then I guess everything
>is fine).

Looking more closely (including looking at the libsasl source),
sasl_decode64() will return SASL_CONTINUE if the input isn't a
complete base-64 block. We get this because we're passing in a 12
character string (say) and a length of 13. I think this is actually
a bug in sm_rrecord(), although it's a bit hard to tell because
there's no commentary about things. Most of the code in there
doesn't care about sm_reply.length, and assumes sm_reply.text is
NUL-terminated. It looks like the sasl library has started caring...

Anyway, I think this probably needs to be fixed by removing my first
attempt and instead making sure that all the code gets the
sm_reply.length correct. (Also I'd like to audit that code to check
that the string really is always NUL-terminated.)

This seems to work, modulo that audit:

===begin===
Index: mts/smtp/smtp.c
===================================================================
RCS file: /cvsroot/nmh/nmh/mts/smtp/smtp.c,v
retrieving revision 1.20
diff -u -r1.20 smtp.c
--- mts/smtp/smtp.c     29 Apr 2008 17:04:38 -0000      1.20
+++ mts/smtp/smtp.c     29 Apr 2008 19:29:03 -0000
@@ -1246,7 +1246,7 @@
            result = sasl_decode64(sm_reply.text, sm_reply.length,
                                   outbuf, sizeof(outbuf), &outlen);
        
-           if (result != SASL_OK && result != SASL_CONTINUE) {
+           if (result != SASL_OK) {
                smtalk(SM_AUTH, "*");
                sm_ierror("SASL base64 decode failed: %s",
                          sasl_errstring(result, NULL, NULL));
@@ -1688,15 +1688,17 @@
 
     fgets (buffer, BUFSIZ, sm_rfp);
     *len = strlen (buffer);
-    if (ferror (sm_rfp) || feof (sm_rfp))
+    /* *len should be >0 except on EOF, but check for safety's sake */
+    if (ferror (sm_rfp) || feof (sm_rfp) || (*len == 0))
        return sm_rerror ();
     if (buffer[*len - 1] != '\n')
        while (getc (sm_rfp) != '\n' && !ferror (sm_rfp) && !feof (sm_rfp))
            continue;
     else
-       if (buffer[*len - 2] == '\r')
+       if ((*len > 1) && (buffer[*len - 2] == '\r'))
            *len -= 1;
-    buffer[*len - 1] = 0;
+    *len -= 1;
+    buffer[*len] = 0;
 
     return OK;
 }
===endit===

>The second ... yeah, that patch should be applied.

Do you want to commit it or shall I?

-- PMM