From: Peter Maydell Date: Wed, 30 Apr 2008 19:07:19 +0000 (+0000) Subject: Port fixes from trunk (SASL fixes and mishandling of reply string buffer) X-Git-Tag: nmh-1_3_RC2~5 X-Git-Url: http://git.marmaro.de/?p=mmh;a=commitdiff_plain;h=70b0eba24703bf7b453259eb862772688d08ee34 Port fixes from trunk (SASL fixes and mishandling of reply string buffer) --- diff --git a/ChangeLog b/ChangeLog index 4e42901..2bd8a63 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2008-04-30 Peter Maydell + + * Fixes ported from trunk: + + * mts/smtp/smtp.c: provide a callback for SASL_CB_AUTHNAME + (fixes issue with SASL sending the wrong username in some + circumstances). Thanks to + for the patch. + + * Revert previous attempt at fix for SASL issue as it + is the wrong approach. + + * Fix in correct manner, by making sm_rrecord() and thus + sm_hear() set the length of the reply string correctly + (the SASL libraries now care if you pass in the wrong + length). + + * Correct various places in smtp.c where the reply string + might not have been correctly NUL-terminated. Includes a + fix for a particularly nasty and long standing screwup + where the buffer length counting in smhear() was totally + broken for continued lines from the server. + 2008-04-29 Peter Maydell * Port fix from trunk for SASL not working with newer libsasl. diff --git a/mts/smtp/smtp.c b/mts/smtp/smtp.c index 9527a5e..35eefb4 100644 --- a/mts/smtp/smtp.c +++ b/mts/smtp/smtp.c @@ -105,6 +105,8 @@ static sasl_callback_t callbacks[] = { #define SM_SASL_N_CB_USER 0 { SASL_CB_PASS, sm_get_pass, NULL }, #define SM_SASL_N_CB_PASS 1 + { SASL_CB_AUTHNAME, sm_get_user, NULL }, +#define SM_SASL_N_CB_AUTHNAME 2 { SASL_CB_LIST_END, NULL, NULL }, }; #endif /* CYRUS_SASL */ @@ -139,6 +141,7 @@ static RETSIGTYPE alrmser (int); static char *EHLOset (char *); #ifdef MPOP +static int sm_perror (char *fmt, ...); /* * smtp.c's own static copy of several nmh library subroutines */ @@ -509,7 +512,7 @@ sm_winit (int mode, char *from) #ifdef MPOP if (sm_ispool && !sm_wfp) { - strlen (strcpy (sm_reply.text, "unable to create new spool file")); + sm_reply.length = strlen (strcpy (sm_reply.text, "unable to create new spool file")); sm_reply.code = NOTOK; return RP_BHST; } @@ -681,7 +684,8 @@ sm_end (int type) case NOTOK: sm_note.code = sm_reply.code; - strncpy (sm_note.text, sm_reply.text, sm_note.length = sm_reply.length);/* fall */ + sm_note.length = sm_reply.length; + memcpy (sm_note.text, sm_reply.text, sm_reply.length + 1);/* fall */ case DONE: if (smtalk (SM_RSET, "RSET") == 250 && type == DONE) return RP_OK; @@ -694,7 +698,8 @@ sm_end (int type) } if (type == NOTOK) { sm_reply.code = sm_note.code; - strncpy (sm_reply.text, sm_note.text, sm_reply.length = sm_note.length); + sm_reply.length = sm_note.length; + memcpy (sm_reply.text, sm_note.text, sm_note.length + 1); } break; } @@ -752,20 +757,7 @@ sm_bulk (char *file) gp = NULL; k = strlen (file) - sizeof(".bulk"); if ((fp = fopen (file, "r")) == NULL) { - int len; - - snprintf (sm_reply.text, sizeof(sm_reply.text), - "unable to read %s: ", file); - bp = sm_reply.text; - len = strlen (bp); - bp += len; - if ((s = strerror (errno))) - strncpy (bp, s, sizeof(sm_reply.text) - len); - else - snprintf (bp, sizeof(sm_reply.text) - len, "Error %d", errno); - sm_reply.length = strlen (sm_reply.text); - sm_reply.code = NOTOK; - return RP_BHST; + return sm_perror("unable to read %s: ", file); } if (sm_debug) { printf ("reading file %s\n", file); @@ -826,17 +818,7 @@ losing2: if ((cc = write (fileno (sm_wfp), dp, i)) == NOTOK) { int len; losing3: - strcpy (sm_reply.text, "error writing to server: ", - sizeof(sm_reply.text)); - bp = sm_reply.text; - len = strlen (bp); - bp += len; - if ((s = strerror (errno))) - strncpy (bp, s, sizeof(sm_reply.text) - len); - else - snprintf (bp, sizeof(sm_reply.text) - len, - "unknown error %d", errno); - sm_reply.length = strlen (sm_reply.text); + sm_perror("error writing to server: "); goto losing2; } else @@ -981,19 +963,7 @@ bad_data: for (dp = cp, i = cc; i > 0; dp += j, i -= j) if ((j = fread (cp, sizeof(*cp), i, fp)) == OK) { if (ferror (fp)) { - int len; - - snprintf (sm_reply.text, sizeof(sm_reply.text), - "error reading %s: ", file); - bp = sm_reply.text; - len = strlen (bp); - bp += len; - if ((s = strerror (errno))) - strncpy (bp, s, sizeof(sm_reply.text) - len); - else - snprintf (bp, sizeof(sm_reply.text) - len, - "unknown error %d", errno); - sm_reply.length = strlen (sm_reply.text); + sm_perror("error reading %s: ", file); goto losing2; } cc = dp - cp; @@ -1116,6 +1086,7 @@ sm_auth_sasl(char *user, char *mechlist, char *host) user = getusername(); callbacks[SM_SASL_N_CB_USER].context = user; + callbacks[SM_SASL_N_CB_AUTHNAME].context = user; /* * This is a _bit_ of a hack ... but if the hostname wasn't supplied @@ -1246,7 +1217,7 @@ sm_auth_sasl(char *user, char *mechlist, char *host) 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)); @@ -1325,7 +1296,7 @@ sm_get_user(void *context, int id, const char **result, unsigned *len) { char *user = (char *) context; - if (! result || id != SASL_CB_USER) + if (! result || ((id != SASL_CB_USER) && (id != SASL_CB_AUTHNAME))) return SASL_BADPARAM; *result = user; @@ -1380,6 +1351,36 @@ sm_ierror (char *fmt, ...) return RP_BHST; } +#ifdef MPOP +static int +sm_perror (char *fmt, ...) +{ + /* Fill in sm_reply with a suitable error string based on errno. + * This isn't particularly MPOP specific, it just happens that that's + * the only code that uses it currently. + */ + char *bp, *s; + int len, eno = errno; + + va_list ap; + va_start(ap,fmt); + vsnprintf (sm_reply.text, sizeof(sm_reply.text), fmt, ap); + va_end(ap); + + bp = sm_reply.text; + len = strlen(bp); + bp += len; + if ((s = strerror(eno))) + snprintf(bp, sizeof(sm_reply.text) - len, "%s", s); + else + snprintf(bp, sizeof(sm_reply.text) - len, "unknown error %d", eno); + + sm_reply.length = strlen (sm_reply.text); + sm_reply.code = NOTOK; + + return RP_BHST; +} +#endif static int smtalk (int time, char *fmt, ...) @@ -1412,22 +1413,7 @@ smtalk (int time, char *fmt, ...) snprintf (file, sizeof(file), "%s%c.bulk", sm_tmpfil, (char) (sm_ispool + 'a' - 1)); if (rename (sm_tmpfil, file) == NOTOK) { - int len; - char *bp; - - snprintf (sm_reply.text, sizeof(sm_reply.text), - "error renaming %s to %s: ", sm_tmpfil, file); - bp = sm_reply.text; - len = strlen (bp); - bp += len; - if ((s = strerror (errno))) - strncpy (bp, s, sizeof(sm_reply.text) - len); - else - snprintf (bp, sizeof(sm_reply.text) - len, - "unknown error %d", errno); - sm_reply.length = strlen (sm_reply.text); - sm_reply.code = NOTOK; - return RP_BHST; + return sm_perror("error renaming %s to %s: ", sm_tmpfil, file); } fclose (sm_wfp); if (sm_wfp = fopen (sm_tmpfil, "w")) @@ -1646,6 +1632,7 @@ again: ; sm_reply.code = code; more = cont; if (bc <= 0) { + /* can never fail to 0-terminate because of size of buffer vs fixed string */ strncpy (buffer, sm_noreply, sizeof(buffer)); bp = buffer; bc = strlen (sm_noreply); @@ -1653,12 +1640,14 @@ again: ; } if ((i = min (bc, rc)) > 0) { - strncpy (rp, bp, i); + memcpy (rp, bp, i); rp += i; rc -= i; - if (more && rc > strlen (sm_moreply) + 1) { - strncpy (sm_reply.text + rc, sm_moreply, sizeof(sm_reply.text) - rc); - rc += strlen (sm_moreply); + i = strlen(sm_moreply); + if (more && rc > i + 1) { + memcpy (rp, sm_moreply, i); /* safe because of check in if() */ + rp += i; + rc -= i; } } if (more) @@ -1672,6 +1661,7 @@ again: ; } sm_reply.length = rp - sm_reply.text; + sm_reply.text[sm_reply.length] = 0; return sm_reply.code; } return NOTOK; @@ -1688,15 +1678,17 @@ sm_rrecord (char *buffer, int *len) 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; }