Actually...
On Thu, Feb 28, 2013 at 22:49:21 +0100, Thomas Preud'homme wrote:
> diff -Nru dspam-3.10.1+dfsg/debian/patches/009_fix_recipient_corruption_when_releasing_message_from_quarantine.diff dspam-3.10.1+dfsg/debian/patches/009_fix_recipient_corruption_when_releasing_message_from_quarantine.diff
> --- dspam-3.10.1+dfsg/debian/patches/009_fix_recipient_corruption_when_releasing_message_from_quarantine.diff 1970-01-01 01:00:00.000000000 +0100
> +++ dspam-3.10.1+dfsg/debian/patches/009_fix_recipient_corruption_when_releasing_message_from_quarantine.diff 2013-02-28 21:34:52.000000000 +0100
> @@ -0,0 +1,55 @@
> +Description: Fix recipient corruption when releasing a message from quarantine
> +
> +When releasing mail from quarantine, dspam corrupts the FROM part in the
> +SMTP/LMTP handshake.
> +
> +Author: Allan Ievers <aimail-dspam_users@rearden.com>
> +Origin: vendor
> +Bug-Debian: http://bugs.debian.org/698136
> +Forwarded: stevan@bajic.ch
> +Last-Update: 2013-02-28
> +
> +diff --git a/src/dspam.c b/src/dspam.c
> +index 26266c9..68e1165 100644
> +--- a/src/dspam.c
> ++++ b/src/dspam.c
> +@@ -498,8 +498,9 @@ process_message (
> + ATX->train_pristine = 1;
> + }
> +
> +- /* Change also the mail recipient */
> +- ATX->recipient = CTX->username;
> ++ /* Change also the mail recipient. ATX->recipient either points to
> ++ * recipient[] or mailbox[] in process_users, hence the size of 256 */
> ++ strlcpy(ATX->recipient, CTX->username, 256);
> +
> + }
> + }
> +@@ -1621,6 +1622,7 @@ int process_users(AGENT_CTX *ATX, buffer *message) {
> + char filename[MAX_FILENAME_LENGTH];
> + int optin, optout;
> + char *username = NULL;
> ++ char recipient[256];
> +
> + /* If ServerParameters specifies a --user, there will only be one
> + * instance on the stack, but possible multiple recipients. So we
> +@@ -1659,7 +1661,7 @@ int process_users(AGENT_CTX *ATX, buffer *message) {
> + username = node_nt->ptr;
> +
> + if (node_rcpt) {
> +- ATX->recipient = node_rcpt->ptr;
> ++ strlcpy(recipient, node_rcpt->ptr, sizeof(recipient));
> + node_rcpt = c_nt_next (ATX->recipients, &c_rcpt);
> + } else {
> +
> +@@ -1667,8 +1669,9 @@ int process_users(AGENT_CTX *ATX, buffer *message) {
> + if (have_rcpts)
> + break;
> +
> +- ATX->recipient = node_nt->ptr;
> ++ strlcpy(recipient, node_nt->ptr, sizeof(recipient));
> + }
> ++ ATX->recipient = recipient;
> +
> + /* If support for "+detail" is enabled, save full mailbox name for
> + delivery and strip detail for processing */
Seems like ATX->recipient now points somewhere on the stack, and thus in
la-la-land at the end of the loop in process_users. Is there any
guarantee it's not reused after that? The scoping is kind of
non-obvious...
Cheers,
Julien
Attachment:
signature.asc
Description: Digital signature