From 4a7912c04ad51fe56092879b350752b3828d990b Mon Sep 17 00:00:00 2001 From: Wietse Venema Date: Wed, 5 Apr 2006 00:00:00 -0500 Subject: [PATCH] postfix-2.3-20060405 --- postfix/HISTORY | 29 +++++++++++++++++++ postfix/src/cleanup/cleanup_message.c | 9 +++--- postfix/src/global/mail_version.h | 2 +- postfix/src/global/mime_state.c | 41 ++++++++++++++++++--------- postfix/src/global/mime_state.h | 2 +- postfix/src/local/maildir.c | 9 ++++-- postfix/src/verify/verify.c | 9 +++++- postfix/src/virtual/maildir.c | 9 ++++-- 8 files changed, 85 insertions(+), 25 deletions(-) diff --git a/postfix/HISTORY b/postfix/HISTORY index 5f96028e9..b03bee8c7 100644 --- a/postfix/HISTORY +++ b/postfix/HISTORY @@ -12092,6 +12092,35 @@ Apologies for any names omitted. Bugfix: the pipe-to-command error message was lost when the command could not be executed. File: global/pipe_command.c. +20060404 + + Bugfix in sanity check: after reading a record from the + address verification database, a sanity check did not reject + a record with all-zero time stamp fields. Such records are + never written; the test is there just in case something is + broken, so that Postfix will not blindly march on and create + chaos. The sanity check tested pointer values, instead of + dereferencing the pointers. Found by Coverity. File: + verify/verify.c. + + Bugfix in sanity check: when the maildir delivery routine + opens an output file it looks up the file attributes via + the file handle it just got. There is a sanity check that + detects if the attribute lookup fails, an error that never + happens. The code that handles the impossible error did not + close the output file. This would cause a virtual or local + delivery agent to waste up to 100 file descriptors. But + for that error to happen the system would have to be so + sick that you would have more serious problems than a file + descriptor leak. Found by Coverity. Files: local/maildir.c, + virtual/maildir.c. + +20060405 + + Bugfix: the MIME parser assumed input is null terminated + when reporting errors. Fix by Leandro Santi. Files: + global/mime_state.c, cleanup/cleanup_message.c. + Wish list: Don't send xforward attributes to every site that announces diff --git a/postfix/src/cleanup/cleanup_message.c b/postfix/src/cleanup/cleanup_message.c index c09764d0f..52ab9c09c 100644 --- a/postfix/src/cleanup/cleanup_message.c +++ b/postfix/src/cleanup/cleanup_message.c @@ -827,7 +827,7 @@ static void cleanup_message_headerbody(CLEANUP_STATE *state, int type, /* cleanup_mime_error_callback - error report call-back routine */ static void cleanup_mime_error_callback(void *context, int err_code, - const char *text) + const char *text, ssize_t len) { CLEANUP_STATE *state = (CLEANUP_STATE *) context; const char *origin; @@ -839,9 +839,10 @@ static void cleanup_mime_error_callback(void *context, int err_code, if ((err_code & ~MIME_ERR_TRUNC_HEADER) != 0) { if ((origin = nvtable_find(state->attr, MAIL_ATTR_ORIGIN)) == 0) origin = MAIL_ATTR_ORG_NONE; - msg_info("%s: reject: mime-error %s: %.100s from %s; from=<%s> to=<%s>", - state->queue_id, mime_state_error(err_code), text, origin, - state->sender, state->recip ? state->recip : "unknown"); +#define TEXT_LEN (len < 100 ? (int) len : 100) + msg_info("%s: reject: mime-error %s: %.*s from %s; from=<%s> to=<%s>", + state->queue_id, mime_state_error(err_code), TEXT_LEN, text, + origin, state->sender, state->recip ? state->recip : "unknown"); } } diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index fc5b98bc3..9069dcd46 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * Patches change both the patchlevel and the release date. Snapshots have no * patchlevel; they change the release date only. */ -#define MAIL_RELEASE_DATE "20060403" +#define MAIL_RELEASE_DATE "20060405" #define MAIL_VERSION_NUMBER "2.3" #ifdef SNAPSHOT diff --git a/postfix/src/global/mime_state.c b/postfix/src/global/mime_state.c index 8988ad5bb..5727ac898 100644 --- a/postfix/src/global/mime_state.c +++ b/postfix/src/global/mime_state.c @@ -391,14 +391,24 @@ static MIME_ENCODING mime_encoding_map[] = { /* RFC 2045 */ #define END(x) vstring_end(x) #define CU_CHAR_PTR(x) ((const unsigned char *) (x)) -#define REPORT_ERROR(state, err_type, text) do { \ +#define REPORT_ERROR_LEN(state, err_type, text, len) do { \ if ((state->err_flags & err_type) == 0) { \ if (state->err_print != 0) \ - state->err_print(state->app_context, err_type, text); \ + state->err_print(state->app_context, err_type, text, len); \ state->err_flags |= err_type; \ } \ } while (0) +#define REPORT_ERROR(state, err_type, text) do { \ + const char *_text = text; \ + ssize_t _len = strlen(text); \ + REPORT_ERROR_LEN(state, err_type, _text, _len); \ + } while (0) + +#define REPORT_ERROR_BUF(state, err_type, buf) \ + REPORT_ERROR_LEN(state, err_type, STR(buf), LEN(buf)) + + /* * Outputs and state changes are interleaved, so we must maintain separate * offsets for header and body segments. @@ -600,8 +610,8 @@ static void mime_state_content_type(MIME_STATE *state, && state->token[1].type == '=') { if (state->nesting_level > var_mime_maxdepth) { if (state->static_flags & MIME_OPT_REPORT_NESTING) - REPORT_ERROR(state, MIME_ERR_NESTING, - STR(state->output_buffer)); + REPORT_ERROR_BUF(state, MIME_ERR_NESTING, + state->output_buffer); } else { mime_state_push(state, def_ctype, def_stype, state->token[2].u.value); @@ -769,8 +779,8 @@ int mime_state_update(MIME_STATE *state, int rec_type, vstring_strncat(state->output_buffer, text, len); } else { if (state->static_flags & MIME_OPT_REPORT_TRUNC_HEADER) - REPORT_ERROR(state, MIME_ERR_TRUNC_HEADER, - STR(state->output_buffer)); + REPORT_ERROR_BUF(state, MIME_ERR_TRUNC_HEADER, + state->output_buffer); } SAVE_PREV_REC_TYPE_AND_RETURN_ERR_FLAGS(state, rec_type); } @@ -780,8 +790,8 @@ int mime_state_update(MIME_STATE *state, int rec_type, vstring_strncat(state->output_buffer, text, len); } else { if (state->static_flags & MIME_OPT_REPORT_TRUNC_HEADER) - REPORT_ERROR(state, MIME_ERR_TRUNC_HEADER, - STR(state->output_buffer)); + REPORT_ERROR_BUF(state, MIME_ERR_TRUNC_HEADER, + state->output_buffer); } SAVE_PREV_REC_TYPE_AND_RETURN_ERR_FLAGS(state, rec_type); } @@ -811,8 +821,8 @@ int mime_state_update(MIME_STATE *state, int rec_type, for (cp = CU_CHAR_PTR(STR(state->output_buffer)); cp < CU_CHAR_PTR(END(state->output_buffer)); cp++) if (*cp & 0200) { - REPORT_ERROR(state, MIME_ERR_8BIT_IN_HEADER, - STR(state->output_buffer)); + REPORT_ERROR_BUF(state, MIME_ERR_8BIT_IN_HEADER, + state->output_buffer); break; } } @@ -836,7 +846,7 @@ int mime_state_update(MIME_STATE *state, int rec_type, /* * See if this input is (the beginning of) a message header. - * + * * Normalize obsolete "name space colon" syntax to "name colon". * Things would be too confusing otherwise. * @@ -990,7 +1000,8 @@ int mime_state_update(MIME_STATE *state, int rec_type, && (state->err_flags & MIME_ERR_8BIT_IN_7BIT_BODY) == 0) { for (cp = CU_CHAR_PTR(text); cp < CU_CHAR_PTR(text + len); cp++) if (*cp & 0200) { - REPORT_ERROR(state, MIME_ERR_8BIT_IN_7BIT_BODY, text); + REPORT_ERROR_LEN(state, MIME_ERR_8BIT_IN_7BIT_BODY, + text, len); break; } } @@ -1137,9 +1148,11 @@ static void body_end(void *context) vstream_fprintf(stream, "BODY END\n"); } -static void err_print(void *unused_context, int err_flag, const char *text) +static void err_print(void *unused_context, int err_flag, + const char *text, ssize_t len) { - msg_warn("%s: %.100s", mime_state_error(err_flag), text); + msg_warn("%s: %.*s", mime_state_error(err_flag), + len < 100 ? (int) len : 100, text); } int var_header_limit = 2000; diff --git a/postfix/src/global/mime_state.h b/postfix/src/global/mime_state.h index 952967b11..78454a23e 100644 --- a/postfix/src/global/mime_state.h +++ b/postfix/src/global/mime_state.h @@ -28,7 +28,7 @@ typedef struct MIME_STATE MIME_STATE; typedef void (*MIME_STATE_HEAD_OUT) (void *, int, HEADER_OPTS *, VSTRING *, off_t); typedef void (*MIME_STATE_BODY_OUT) (void *, int, const char *, ssize_t, off_t); typedef void (*MIME_STATE_ANY_END) (void *); -typedef void (*MIME_STATE_ERR_PRINT) (void *, int, const char *); +typedef void (*MIME_STATE_ERR_PRINT) (void *, int, const char *, ssize_t); extern MIME_STATE *mime_state_alloc(int, MIME_STATE_HEAD_OUT, MIME_STATE_ANY_END, MIME_STATE_BODY_OUT, MIME_STATE_ANY_END, MIME_STATE_ERR_PRINT, void *); extern int mime_state_update(MIME_STATE *, int, const char *, ssize_t); diff --git a/postfix/src/local/maildir.c b/postfix/src/local/maildir.c index 08a50b43f..2860f7769 100644 --- a/postfix/src/local/maildir.c +++ b/postfix/src/local/maildir.c @@ -191,8 +191,13 @@ int deliver_maildir(LOCAL_STATE state, USER_ATTR usr_attr, char *path) dsb_simple(why, mbox_dsn(errno, "5.2.0"), "create maildir file %s: %m", tmpfile); } else if (fstat(vstream_fileno(dst), &st) < 0) { - dsb_simple(why, mbox_dsn(errno, "5.2.0"), - "create maildir file %s: %m", tmpfile); + + /* + * Coverity 200604: file descriptor leak in code that never executes. + * Code replaced by msg_fatal(), as it is not worthwhile to continue + * after an impossible error condition. + */ + msg_fatal("fstat %s: %m", tmpfile); } else { vstring_sprintf(buf, "%lu.V%lxI%lxM%lu.%s", (unsigned long) starttime.tv_sec, diff --git a/postfix/src/verify/verify.c b/postfix/src/verify/verify.c index 49fccbcb1..b656a17d1 100644 --- a/postfix/src/verify/verify.c +++ b/postfix/src/verify/verify.c @@ -266,11 +266,18 @@ static int verify_parse_entry(char *buf, int *status, long *probed, *probed = atol(probed_text); *updated = atol(updated_text); *status = atoi(buf); + + /* + * Coverity 200604: the code incorrectly tested (probed || updated), + * so that the sanity check never detected all-zero time stamps. Such + * records are never written. If we read a record with all-zero time + * stamps, then something is badly broken. + */ if ((*status == DEL_RCPT_STAT_OK || *status == DEL_RCPT_STAT_DEFER || *status == DEL_RCPT_STAT_BOUNCE || *status == DEL_RCPT_STAT_TODO) - && (probed || updated)) + && (*probed || *updated)) return (0); } msg_warn("bad address verify table entry: %.100s", buf); diff --git a/postfix/src/virtual/maildir.c b/postfix/src/virtual/maildir.c index 48b7b7fa9..8fcccae68 100644 --- a/postfix/src/virtual/maildir.c +++ b/postfix/src/virtual/maildir.c @@ -185,8 +185,13 @@ int deliver_maildir(LOCAL_STATE state, USER_ATTR usr_attr) dsb_simple(why, mbox_dsn(errno, "4.2.0"), "create maildir file %s: %m", tmpfile); } else if (fstat(vstream_fileno(dst), &st) < 0) { - dsb_simple(why, mbox_dsn(errno, "4.2.0"), - "create maildir file %s: %m", tmpfile); + + /* + * Coverity 200604: file descriptor leak in code that never executes. + * Code replaced by msg_fatal(), as it is not worthwhile to continue + * after an impossible error condition. + */ + msg_fatal("fstat %s: %m", tmpfile); } else { vstring_sprintf(buf, "%lu.V%lxI%lxM%lu.%s", (unsigned long) starttime.tv_sec, -- 2.47.3