From eab49ebd857dfee741125c50b60ce5ff41e8c3d0 Mon Sep 17 00:00:00 2001 From: David Schmidt Date: Tue, 5 Oct 2004 02:03:19 +0000 Subject: [PATCH] Add the ability to check jpeg images for invalid lengths of comment blocks. Defensive strategy against the exploit: Microsoft Security Bulletin MS04-028 Buffer Overrun in JPEG Processing (GDI+) Could Allow Code Execution (833987) Enabled with +inspect-jpegs in actions files. --- default.action.master | 6 +- src/actionlist.h | 4 ++ src/deanimate.c | 126 +++++++++++++++++++++++++++++++-- src/deanimate.h | 20 ++++-- src/filters.c | 86 ++++++++++++++++++++-- src/filters.h | 10 ++- src/jcc.c | 21 +++++- src/parsers.c | 9 ++- src/project.h | 11 ++- templates/edit-actions-for-url | 18 +++++ 10 files changed, 288 insertions(+), 23 deletions(-) diff --git a/default.action.master b/default.action.master index 87b0cb95..a218c6d6 100644 --- a/default.action.master +++ b/default.action.master @@ -2,7 +2,7 @@ # # File : $Source: /cvsroot/ijbswa/current/default.action.master,v $ # -# $Id: default.action.master,v 1.7 2003/02/20 14:00:55 hal9 Exp $ +# $Id: default.action.master,v 1.8 2003/09/22 00:33:01 david__schmidt Exp $ # # Purpose : Default actions file, see # http://www.privoxy.org/user-manual/actions-file.html @@ -283,6 +283,9 @@ # in which case a "blocked" image can be sent rather than a HTML page. # See +set-image-blocker{} for the control over what is actually sent. # +# +inspect-jpegs +# Scan jpeg headers for malformed comment blocks and correct them. +# # +set-image-blocker{blank} # +set-image-blocker{pattern} # +set-image-blocker{custom} @@ -443,6 +446,7 @@ allow-ads = -block -filter{banners-by-size} -filter{banners-by-link} +hide-from-header{block} \ +hide-referrer{forge} \ -hide-user-agent \ ++inspect-jpegs \ -kill-popups \ -limit-connect \ +prevent-compression \ diff --git a/src/actionlist.h b/src/actionlist.h index 9b344ef9..ce5f5fea 100644 --- a/src/actionlist.h +++ b/src/actionlist.h @@ -39,6 +39,9 @@ * * Revisions : * $Log: actionlist.h,v $ + * Revision 2.2 2002/09/12 14:05:14 oes + * Added more aliases for prehistoric action names + * * Revision 2.1 2002/09/04 14:52:18 oes * Synced with the stable branch: * Revision 1.17.2.1 2002/08/02 12:50:47 oes @@ -132,6 +135,7 @@ DEFINE_CGI_PARAM_RADIO ("hide-referrer", ACTION_HIDE_REFERER, DEFINE_CGI_PARAM_CUSTOM ("hide-referrer", ACTION_HIDE_REFERER, ACTION_STRING_REFERER, "http://www.google.com/") DEFINE_ACTION_STRING ("hide-user-agent", ACTION_HIDE_USER_AGENT, ACTION_STRING_USER_AGENT) DEFINE_CGI_PARAM_NO_RADIO("hide-user-agent", ACTION_HIDE_USER_AGENT, ACTION_STRING_USER_AGENT, "Privoxy/3.0 (Anonymous)") +DEFINE_ACTION_BOOL ("inspect-jpegs", ACTION_JPEG_INSPECT) DEFINE_ACTION_BOOL ("kill-popups", ACTION_NO_POPUPS) DEFINE_ACTION_STRING ("limit-connect", ACTION_LIMIT_CONNECT, ACTION_STRING_LIMIT_CONNECT) DEFINE_CGI_PARAM_NO_RADIO("limit-connect", ACTION_LIMIT_CONNECT, ACTION_STRING_LIMIT_CONNECT, "443") diff --git a/src/deanimate.c b/src/deanimate.c index 55f323b4..b411f90c 100644 --- a/src/deanimate.c +++ b/src/deanimate.c @@ -1,16 +1,19 @@ -const char deanimate_rcs[] = "$Id: deanimate.c,v 2.0 2002/06/04 14:34:21 jongfoster Exp $"; +const char deanimate_rcs[] = "$Id: deanimate.c,v 2.1 2002/08/26 11:06:27 sarantis Exp $"; /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/src/deanimate.c,v $ * - * Purpose : Declares functions to deanimate GIF images on the fly. + * Purpose : Declares functions to manipulate binary images on the + * fly. High-level functions include: + * - Deanimation of GIF images + * - Fixup of malformed comment block in JPEG headers * * Functions declared include: gif_deanimate, buf_free, - * buf_copy, buf_getbyte, gif_skip_data_block, and - * gif_extract_image + * buf_copy, buf_getbyte, gif_skip_data_block, + * gif_extract_image and jpeg_inspect * - * Copyright : Written by and Copyright (C) 2001 by the the SourceForge - * Privoxy team. http://www.privoxy.org/ + * Copyright : Written by and Copyright (C) 2001 - 2004 by the the + * SourceForge Privoxy team. http://www.privoxy.org/ * * Based on the GIF file format specification (see * http://tronche.com/computer-graphics/gif/gif89a.html) @@ -37,6 +40,9 @@ const char deanimate_rcs[] = "$Id: deanimate.c,v 2.0 2002/06/04 14:34:21 jongfos * * Revisions : * $Log: deanimate.c,v $ + * Revision 2.1 2002/08/26 11:06:27 sarantis + * Fix typo failiure -> failure + * * Revision 2.0 2002/06/04 14:34:21 jongfoster * Moving source files to src/ * @@ -86,6 +92,7 @@ const char deanimate_rcs[] = "$Id: deanimate.c,v 2.0 2002/06/04 14:34:21 jongfos #include #include +#include "errlog.h" #include "project.h" #include "deanimate.h" #include "miscutil.h" @@ -503,6 +510,113 @@ write: } +/********************************************************************* + * + * Function : jpeg_inspect + * + * Description : Checks a jpeg image for an invalid length in a + * comment block (0xFFFE0000 or 0xFFFE0001) and + * changes it to 0xFFFE0002. Defensive strategy + * against the exploit: + * Microsoft Security Bulletin MS04-028 + * Buffer Overrun in JPEG Processing (GDI+) Could + * Allow Code Execution (833987) + * + * Parameters : + * 1 : src = Pointer to the image binbuffer + * + * Returns : 0 on success, or 1 on failure + * + *********************************************************************/ +int jpeg_inspect(struct binbuffer *src, struct binbuffer *dst) +{ + long i; + /* + * We process the image using a simple finite state machine, + * searching for byte patterns. + */ + enum { J_INIT, /* The initial state */ + J_FF, /* Found byte 0xFF */ + J_FE, /* Found bytes 0xFF 0xFE */ + J_00, /* Found bytes 0xFF 0xFE 0x00 */ + J_DA /* + * Found bytes 0xFF 0xDA; short-circuit to done-ness + * since this signals the beginning end of headers. + */ + }; + short state = J_INIT; + unsigned char c; + + if (NULL == src || NULL == dst) + { + return 1; + } + + if (buf_copy(src, dst, src->size)) + { + return 1; + } + + /* Need to search the jpg for patterns: + * 0xFF 0xFE 0x00 0x00 + * or + * 0xFF 0xFE 0x00 0x01 + * from beginning until: + * 0xFF 0xDA + * (or the end of the buffer) + * If found, change the pattern to 0xFF 0xFE 0x00 0x02 + */ + + for (i = 0; i < dst->size; i++) + { + c = dst->buffer[i]; + switch (state) + { + case J_INIT: + if (c == 0xFF) + state = J_FF; + break; + case J_FF: + if (c == 0xDA) + state = J_DA; /* End of headers - we're done with this image. */ + else if (c == 0xFE) + state = J_FE; + else + state = J_INIT; + break; + case J_FE: + if (c == 0x00) + state = J_00; + else + state = J_INIT; + break; + case J_00: + if ((c == 0x00) || (c == 0x01)) + { + dst->buffer[i] = 2; /* Reset comment block size to 2. */ + log_error(LOG_LEVEL_INFO, "JPEG comment exploit removed."); + /* TODO: + * I'm unsure if we can have more than one comment block. Just in case, + * we'll scan the rest of the header for more by going back to J_INIT + * state. If there is no possibility of >1 comment block, we could + * short-circuit to done-ness here. + */ + state = J_INIT; + } + else + state = J_INIT; + break; + default: + break; + } + if (state == J_DA) + break; + } + + return 0; +} + + /* Local Variables: tab-width: 3 diff --git a/src/deanimate.h b/src/deanimate.h index 1e8d60f5..f0aed467 100644 --- a/src/deanimate.h +++ b/src/deanimate.h @@ -1,17 +1,21 @@ #ifndef DEANIMATE_H_INCLUDED #define DEANIMATE_H_INCLUDED -#define DEANIMATE_H_VERSION "$Id: deanimate.h,v 1.8 2002/03/26 22:29:54 swa Exp $" +#define DEANIMATE_H_VERSION "$Id: deanimate.h,v 2.0 2002/06/04 14:34:21 jongfoster Exp $" /********************************************************************* * - * File : $Source: /cvsroot/ijbswa/current/deanimate.h,v $ + * File : $Source: /cvsroot/ijbswa/current/src/deanimate.h,v $ * - * Purpose : Declares functions to deanimate GIF images on the fly. + * Purpose : Declares functions to manipulate binary images on the + * fly. High-level functions include: + * - Deanimation of GIF images + * - Fixup of malformed comment block in JPEG headers * - * Functions declared include: gif_deanimate, buf_free + * Functions declared include: gif_deanimate, buf_free, + * jpeg_inspect * * - * Copyright : Written by and Copyright (C) 2001 Andreas S. Oesterhelt - * for the Privoxy team. http://www.privoxy.org/ + * Copyright : Written by and Copyright (C) 2001 - 2004 by the the + * SourceForge Privoxy team. http://www.privoxy.org/ * * Based on ideas from the Image::DeAnim Perl module by * Ken MacFarlane, @@ -40,6 +44,9 @@ * * Revisions : * $Log: deanimate.h,v $ + * Revision 2.0 2002/06/04 14:34:21 jongfoster + * Moving source files to src/ + * * Revision 1.8 2002/03/26 22:29:54 swa * we have a new homepage! * @@ -84,6 +91,7 @@ struct binbuffer * Function prototypes */ extern int gif_deanimate(struct binbuffer *src, struct binbuffer *dst, int get_first_image); +extern int jpeg_inspect(struct binbuffer *src, struct binbuffer *dst); extern void buf_free(struct binbuffer *buf); /* diff --git a/src/filters.c b/src/filters.c index d590b19a..55d946e7 100644 --- a/src/filters.c +++ b/src/filters.c @@ -1,4 +1,4 @@ -const char filters_rcs[] = "$Id: filters.c,v 2.5 2003/09/22 00:33:01 david__schmidt Exp $"; +const char filters_rcs[] = "$Id: filters.c,v 2.6 2003/09/25 02:37:32 david__schmidt Exp $"; /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/src/filters.c,v $ @@ -9,9 +9,10 @@ const char filters_rcs[] = "$Id: filters.c,v 2.5 2003/09/22 00:33:01 david__schm * `block_url', `url_actions', `domain_split', * `filter_popups', `forward_url', 'redirect_url', * `ij_untrusted_url', `intercept_url', `pcrs_filter_respose', - * 'ijb_send_banner', and `trust_url' + * `ijb_send_banner', `trust_url', `gif_deanimate_response', + * `jpeg_inspect_response' * - * Copyright : Written by and Copyright (C) 2001 the SourceForge + * Copyright : Written by and Copyright (C) 2001, 2004 the SourceForge * Privoxy team. http://www.privoxy.org/ * * Based on the Internet Junkbuster originally written @@ -38,6 +39,9 @@ const char filters_rcs[] = "$Id: filters.c,v 2.5 2003/09/22 00:33:01 david__schm * * Revisions : * $Log: filters.c,v $ + * Revision 2.6 2003/09/25 02:37:32 david__schmidt + * Feature request 595948: Re-Filter logging in single line + * * Revision 2.5 2003/09/22 00:33:01 david__schmidt * Enable sending a custom 'blocked' image. Shows up as * "image-blocker-custom-file" parameter in config, and @@ -1305,7 +1309,7 @@ int is_untrusted_url(struct client_state *csp) * * Function : pcrs_filter_response * - * Description : Ecexute all text substitutions from all applying + * Description : Execute all text substitutions from all applying * +filter actions on the text buffer that's been accumulated * in csp->iob->buf. If this changes the contents, set * csp->content_length to the modified size and raise the @@ -1497,6 +1501,80 @@ char *gif_deanimate_response(struct client_state *csp) } +/********************************************************************* + * + * Function : jpeg_inspect_response + * + * Description : + * + * Parameters : + * 1 : csp = Current client state (buffers, headers, etc...) + * + * Returns : a pointer to the (newly allocated) modified buffer + * or NULL in case something went wrong. + * + *********************************************************************/ +char *jpeg_inspect_response(struct client_state *csp) +{ + struct binbuffer *in = NULL, *out = NULL; + char *p = NULL; + size_t size = csp->iob->eod - csp->iob->cur; + + /* + * If the body has a "chunked" transfer-encoding, + * get rid of it first, adjusting size and iob->eod + */ + if (csp->flags & CSP_FLAG_CHUNKED) + { + log_error(LOG_LEVEL_DEANIMATE, "Need to de-chunk first"); + if (0 == (size = remove_chunked_transfer_coding(csp->iob->cur, size))) + { + return(NULL); + } + csp->iob->eod = csp->iob->cur + size; + csp->flags |= CSP_FLAG_MODIFIED; + } + + if (NULL == (in = (struct binbuffer *)zalloc(sizeof *in ))) + { + log_error(LOG_LEVEL_DEANIMATE, "failed! (jpeg no mem 1)"); + return NULL; + } + + if (NULL == (out = (struct binbuffer *)zalloc(sizeof *out))) + { + log_error(LOG_LEVEL_DEANIMATE, "failed! (jpeg no mem 2)"); + return NULL; + } + + in->buffer = csp->iob->cur; + in->size = size; + + /* + * Calling jpeg_inspect has the side-effect of creating and + * modifying the image buffer of "out" directly. + */ + if (jpeg_inspect(in, out)) + { + log_error(LOG_LEVEL_DEANIMATE, "failed! (jpeg parsing)"); + free(in); + buf_free(out); + return(NULL); + + } + else + { + csp->content_length = out->offset; + csp->flags |= CSP_FLAG_MODIFIED; + p = out->buffer; + free(in); + free(out); + return(p); + } + +} + + /********************************************************************* * * Function : remove_chunked_transfer_coding diff --git a/src/filters.h b/src/filters.h index 33f4e49b..026860ac 100644 --- a/src/filters.h +++ b/src/filters.h @@ -1,9 +1,9 @@ #ifndef FILTERS_H_INCLUDED #define FILTERS_H_INCLUDED -#define FILTERS_H_VERSION "$Id: filters.h,v 1.20 2002/04/02 14:56:16 oes Exp $" +#define FILTERS_H_VERSION "$Id: filters.h,v 2.0 2002/06/04 14:34:21 jongfoster Exp $" /********************************************************************* * - * File : $Source: /cvsroot/ijbswa/current/filters.h,v $ + * File : $Source: /cvsroot/ijbswa/current/src/filters.h,v $ * * Purpose : Declares functions to parse/crunch headers and pages. * Functions declared include: @@ -12,7 +12,7 @@ * `ij_untrusted_url', `intercept_url', `re_process_buffer', * `show_proxy_args', and `trust_url' * - * Copyright : Written by and Copyright (C) 2001 the SourceForge + * Copyright : Written by and Copyright (C) 2001, 2004 the SourceForge * Privoxy team. http://www.privoxy.org/ * * Based on the Internet Junkbuster originally written @@ -39,6 +39,9 @@ * * Revisions : * $Log: filters.h,v $ + * Revision 2.0 2002/06/04 14:34:21 jongfoster + * Moving source files to src/ + * * Revision 1.20 2002/04/02 14:56:16 oes * Bugfix: is_untrusted_url() and trust_url() now depend on FEATURE_TRUST, not FEATURE_COOKIE_JAR * @@ -257,6 +260,7 @@ extern const struct forward_spec *forward_url(struct http_request *http, struct */ extern char *pcrs_filter_response(struct client_state *csp); extern char *gif_deanimate_response(struct client_state *csp); +extern char *jpeg_inspect_response(struct client_state *csp); extern int remove_chunked_transfer_coding(char *buffer, const size_t size); /* diff --git a/src/jcc.c b/src/jcc.c index 29b712e3..47b4c83b 100644 --- a/src/jcc.c +++ b/src/jcc.c @@ -1,4 +1,4 @@ -const char jcc_rcs[] = "$Id: jcc.c,v 2.6 2003/06/24 12:24:24 oes Exp $"; +const char jcc_rcs[] = "$Id: jcc.c,v 2.7 2003/09/25 01:44:33 david__schmidt Exp $"; /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/src/jcc.c,v $ @@ -33,6 +33,13 @@ const char jcc_rcs[] = "$Id: jcc.c,v 2.6 2003/06/24 12:24:24 oes Exp $"; * * Revisions : * $Log: jcc.c,v $ + * Revision 2.7 2003/09/25 01:44:33 david__schmidt + * Resyncing HEAD with v_3_0_branch for two OSX fixes: + * Making thread IDs look sane in the logfile for Mach kernels, + * and fixing multithreading crashes due to thread-unsafe + * system calls. + * and + * * Revision 2.6 2003/06/24 12:24:24 oes * Added a line plus Fix-me as a reminder to fix broken force handling in trunk. Thanks to lionel for the hint * @@ -1726,6 +1733,7 @@ static jb_err relay_server_traffic( struct client_state *csp ) int pcrs_filter; /* bool, 1==will filter through pcrs */ int gif_deanimate; /* bool, 1==will deanimate gifs */ + int jpeg_inspect; /* bool, 1==will inspect jpegs */ #ifdef FEATURE_KILL_POPUPS block_popups = ((csp->action->flags & ACTION_NO_POPUPS) != 0); @@ -1736,6 +1744,7 @@ static jb_err relay_server_traffic( struct client_state *csp ) gif_deanimate = ((csp->action->flags & ACTION_DEANIMATE) != 0); + jpeg_inspect = ((csp->action->flags & ACTION_JPEG_INSPECT) != 0); fflush (0); len = read_socket (csp->sfd, buf, sizeof (buf) - 1); @@ -2029,6 +2038,16 @@ static jb_err relay_server_traffic( struct client_state *csp ) { csp->content_filter = gif_deanimate_response; } + + /* Buffer and jpg_inspect this if appropriate. */ + + if ((csp->content_type & CT_JPEG) && /* It's an image/jpeg MIME-Type */ + !csp->http->ssl && /* We talk plaintext */ + jpeg_inspect) /* Policy allows */ + { + csp->content_filter = jpeg_inspect_response; + } + /* * Only write if we're not buffering for content modification */ diff --git a/src/parsers.c b/src/parsers.c index a4fd3523..e8ef114d 100644 --- a/src/parsers.c +++ b/src/parsers.c @@ -1,4 +1,4 @@ -const char parsers_rcs[] = "$Id: parsers.c,v 2.5 2003/09/25 01:44:33 david__schmidt Exp $"; +const char parsers_rcs[] = "$Id: parsers.c,v 2.6 2003/10/02 19:41:23 david__schmidt Exp $"; /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/src/parsers.c,v $ @@ -40,6 +40,11 @@ const char parsers_rcs[] = "$Id: parsers.c,v 2.5 2003/09/25 01:44:33 david__schm * * Revisions : * $Log: parsers.c,v $ + * Revision 2.6 2003/10/02 19:41:23 david__schmidt + * Updated header debug logging to show the header text that is + * being crunched; refactored functions in parsers.c to have a + * single, common exit point + * * Revision 2.5 2003/09/25 01:44:33 david__schmidt * Resyncing HEAD with v_3_0_branch for two OSX fixes: * Making thread IDs look sane in the logfile for Mach kernels, @@ -844,6 +849,8 @@ jb_err server_content_type(struct client_state *csp, char **header) csp->content_type = CT_TEXT; else if (strstr(*header, " image/gif")) csp->content_type = CT_GIF; + else if (strstr(*header, " image/jpeg")) + csp->content_type = CT_JPEG; else csp->content_type = 0; } diff --git a/src/project.h b/src/project.h index 17e51ce9..dd775e30 100644 --- a/src/project.h +++ b/src/project.h @@ -1,7 +1,7 @@ #ifndef PROJECT_H_INCLUDED #define PROJECT_H_INCLUDED /** Version string. */ -#define PROJECT_H_VERSION "$Id: project.h,v 2.4 2002/12/28 03:58:19 david__schmidt Exp $" +#define PROJECT_H_VERSION "$Id: project.h,v 2.5 2003/09/22 00:33:01 david__schmidt Exp $" /********************************************************************* * * File : $Source: /cvsroot/ijbswa/current/src/project.h,v $ @@ -37,6 +37,11 @@ * * Revisions : * $Log: project.h,v $ + * Revision 2.5 2003/09/22 00:33:01 david__schmidt + * Enable sending a custom 'blocked' image. Shows up as + * "image-blocker-custom-file" parameter in config, and + * "+set-image-blocker{custom}" in action files. + * * Revision 2.4 2002/12/28 03:58:19 david__schmidt * Initial drop of dashboard instrumentation - enabled with * --enable-activity-console @@ -831,6 +836,8 @@ struct iob Suitable for GIF filtering. */ #define CT_TABOO 4 /**< csp->content_type bitmask: DO NOT filter, irrespective of other flags. */ +#define CT_JPEG 8 /**< csp->content_type bitmask: + Suitable for JPEG filtering. */ /** * The mask which includes all actions. @@ -876,6 +883,8 @@ struct iob #define ACTION_VANILLA_WAFER 0x00008000UL /** Action bitmap: Limit CONNECT requests to safe ports. */ #define ACTION_LIMIT_CONNECT 0x00010000UL +/** Action bitmap: Inspect if it's a JPEG. */ +#define ACTION_JPEG_INSPECT 0x00020000UL /** Action string index: How to deanimate GIFs */ #define ACTION_STRING_DEANIMATE 0 diff --git a/templates/edit-actions-for-url b/templates/edit-actions-for-url index e0f15087..0548bf44 100644 --- a/templates/edit-actions-for-url +++ b/templates/edit-actions-for-url @@ -32,6 +32,11 @@ # # Revisions : # $Log: edit-actions-for-url,v $ +# Revision 1.31 2003/09/22 00:33:01 david__schmidt +# Enable sending a custom 'blocked' image. Shows up as +# "image-blocker-custom-file" parameter in config, and +# "+set-image-blocker{custom}" in action files. +# # Revision 1.30 2002/09/05 16:12:02 oes # Synced with stable branch: # Revision 1.29.2.3 2002/08/23 02:22:53 hal9 @@ -668,6 +673,19 @@ function show_send_wafer_opts(tf) + + + + + inspect-jpegs + Checks jpeg images for malicious content. +