You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2004/01/03 17:45:21 UTC

Re: cvs commit: httpd-2.0/server gen_test_char.c

On Sat, Jan 03, 2004 at 04:31:32PM -0000, ben@apache.org wrote:
> ben         2004/01/03 08:31:32
> 
>   Modified:    server   gen_test_char.c
>   Log:
>   Make forensic logging safe for POST data. The issue with strchr and NUL is
>   a red herring.

I don't think this is a safe change: 0 is now flagged with
T_ESCAPE_FORENSIC|T_ESCAPE_LOGITEM|T_HTTP_TOKEN_STOP|T_ESCAPE_SHELL_CMD.  
At least ap_find_token() assumes that 0 is not flagged with
T_HTTP_TOKEN_STOP.

>   
>   Revision  Changes    Path
>   1.19      +3 -7      httpd-2.0/server/gen_test_char.c
>   
>   Index: gen_test_char.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/gen_test_char.c,v
>   retrieving revision 1.18
>   retrieving revision 1.19
>   diff -u -r1.18 -r1.19
>   --- gen_test_char.c	3 Jan 2004 15:33:41 -0000	1.18
>   +++ gen_test_char.c	3 Jan 2004 16:31:32 -0000	1.19
>   @@ -90,8 +90,7 @@
>               "#define T_ESCAPE_LOGITEM       (%u)\n"
>               "#define T_ESCAPE_FORENSIC      (%u)\n"
>               "\n"
>   -           "static const unsigned char test_char_table[256] = {\n"
>   -           "    0,",
>   +           "static const unsigned char test_char_table[256] = {",
>               T_ESCAPE_SHELL_CMD,
>               T_ESCAPE_PATH_SEGMENT,
>               T_OS_ESCAPE_PATH,
>   @@ -99,10 +98,7 @@
>               T_ESCAPE_LOGITEM,
>               T_ESCAPE_FORENSIC);
>    
>   -    /* we explicitly dealt with NUL above
>   -     * in case some strchr() do bogosity with it */
>   -
>   -    for (c = 1; c < 256; ++c) {
>   +    for (c = 0; c < 256; ++c) {
>            flags = 0;
>            if (c % 20 == 0)
>                printf("\n    ");
>   @@ -154,7 +150,7 @@
>             * :, | (used as delimiters) and % (used for escaping).
>             */
>            if (!apr_isprint(c) || c == ':' || c == '|' || c == '%'
>   -            || apr_iscntrl(c)) {
>   +            || apr_iscntrl(c) || !c) {
>                flags |= T_ESCAPE_FORENSIC;
>            }
>    
>   
>   
>   

Re: cvs commit: httpd-2.0/server gen_test_char.c

Posted by Ben Laurie <be...@algroup.co.uk>.
Joe Orton wrote:

> On Sat, Jan 03, 2004 at 04:31:32PM -0000, ben@apache.org wrote:
> 
>>ben         2004/01/03 08:31:32
>>
>>  Modified:    server   gen_test_char.c
>>  Log:
>>  Make forensic logging safe for POST data. The issue with strchr and NUL is
>>  a red herring.
> 
> 
> I don't think this is a safe change: 0 is now flagged with
> T_ESCAPE_FORENSIC|T_ESCAPE_LOGITEM|T_HTTP_TOKEN_STOP|T_ESCAPE_SHELL_CMD.  
> At least ap_find_token() assumes that 0 is not flagged with
> T_HTTP_TOKEN_STOP.

I shall fix it.

Cheers,

Ben.


-- 
http://www.apache-ssl.org/ben.html       http://www.thebunker.net/

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff