commit 589d2ed4283130108df5495b5510d822282e1300
Author: Oswald Buddenhagen <o...@users.sf.net>
Date:   Wed Apr 14 16:58:27 2021 +0200

    CVE-2021-3578: fix handling of unexpected APPENDUID response code
    
    if the code was sent in response to anything but a STORE, we'd overwrite
    a data pointer in one of our imap_cmd subclasses, an allocator data
    structure, or the start of the next allocation, with an int that was
    completely under the server's control. it's plausible that this could be
    exploited for remote code execution.
    
    to avoid this, we could ensure that the object is of the right type
    prior to casting, by using a new flag in the parameter block. but it's
    easier to just dispose of the out_uid field altogether and reuse the uid
    field that is present in the parameter block anyway, but was used only
    for FETCH commands so far.
    
    this problem was found by Lukas Braun <ko...@moshbit.net> using a
    fuzzer.

 src/drv_imap.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/drv_imap.c b/src/drv_imap.c
index fbe2fed..4cc3b2a 100644
--- a/src/drv_imap.c
+++ b/src/drv_imap.c
@@ -181,7 +181,6 @@ typedef struct {
        imap_cmd_t gen;
        void (*callback)( int sts, uint uid, void *aux );
        void *callback_aux;
-       uint out_uid;
 } imap_cmd_out_uid_t;
 
 typedef struct {
@@ -1184,11 +1183,22 @@ parse_response_code( imap_store_t *ctx, imap_cmd_t 
*cmd, char *s )
                 */
                for (; isspace( (uchar)*p ); p++);
                error( "*** IMAP ALERT *** %s\n", p );
-       } else if (cmd && !strcmp( "APPENDUID", arg )) {
+       } else if (!strcmp( "APPENDUID", arg )) {
+               // The checks ensure that:
+               // - cmd => this is the final tagged response of a command, at 
which
+               //   point cmd was already removed from ctx->in_progress, so 
param.uid
+               //   is available for reuse.
+               // - !param.uid => the command isn't actually a FETCH. This 
doesn't
+               //   really matter, as the field is safe to overwrite given the
+               //   previous condition; it just has no effect for non-APPENDs.
+               if (!cmd || cmd->param.uid) {
+                       error( "IMAP error: unexpected APPENDUID status\n" );
+                       return RESP_CANCEL;
+               }
                if (!(arg = next_arg( &s )) ||
                    (ctx->uidvalidity = strtoul( arg, &earg, 10 ), *earg) ||
                    !(arg = next_arg( &s )) ||
-                   (((imap_cmd_out_uid_t *)cmd)->out_uid = strtoul( arg, 
&earg, 10 ), *earg))
+                   (cmd->param.uid = strtoul( arg, &earg, 10 ), *earg))
                {
                        error( "IMAP error: malformed APPENDUID status\n" );
                        return RESP_CANCEL;
@@ -2957,7 +2967,6 @@ imap_store_msg( store_t *gctx, msg_data_t *data, int 
to_trash,
        ctx->buffer_mem += data->len;
        cmd->gen.param.data_len = data->len;
        cmd->gen.param.data = data->data;
-       cmd->out_uid = 0;
 
        if (to_trash) {
                cmd->gen.param.create = 1;
@@ -2990,7 +2999,7 @@ imap_store_msg_p2( imap_store_t *ctx ATTR_UNUSED, 
imap_cmd_t *cmd, int response
        imap_cmd_out_uid_t *cmdp = (imap_cmd_out_uid_t *)cmd;
 
        transform_msg_response( &response );
-       cmdp->callback( response, cmdp->out_uid, cmdp->callback_aux );
+       cmdp->callback( response, cmdp->gen.param.uid, cmdp->callback_aux );
 }
 
 /******************* imap_find_new_msgs *******************/


_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to