httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dirk-Willem van Gulik <di...@webweaving.org>
Subject Re: Buffer size in mod_session_crypto.c, decrypt_string()
Date Thu, 19 Nov 2015 09:14:33 GMT

> On 19 Nov 2015, at 10:07, Ewald Dieterich <ewald@mailbox.org> wrote:
> 
> This is from mod_session_crypto.c, decrypt_string():
> 
>    /* strip base64 from the string */
>    decoded = apr_palloc(r->pool, apr_base64_decode_len(in));
>    decodedlen = apr_base64_decode(decoded, in);
>    decoded[decodedlen] = '\0';
> 
> Shouldn't that be ("+ 1" for the added '\0'):
> 
>   decoded = apr_palloc(r->pool, apr_base64_decode_len(in) + 1);
> 
> At least that's how it's done in eg. mod_auth_basic.c. Or can we make any assumptions
about the number of characters that apr_base64_decode_len() returns?
> 

Hmm - very strong feeling of a deja vu:  https://bz.apache.org/bugzilla/show_bug.cgi?id=43448
<https://bz.apache.org/bugzilla/show_bug.cgi?id=43448>  from some 10 years ago (it blew
something out of the water back then).

And pretty sure we fixed the documentation. _len() returns the max size of the buffer (i.e.
+ 1). 

And _decode the exact length of the string without the trailing \0 which it silently does
add. And which may be shorter than _len.

So above should be fine — but the 

>    decoded[decodedlen] = '\0’;

seems both confusing and superfluous. Alternatively one could use _decode_binary()  in which
case above terminator is absolutely needed. There is no _binary_len().

Dw





Mime
View raw message