> On 19 Nov 2015, at 10:07, Ewald Dieterich <[email protected]> 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
