Hello.

If I am not mistaken, there is an undefined behavior in apr_pbase64_encode()
function:

    encoded = (char *) apr_palloc(p, apr_base64_encode_len(l));
    l = apr_base64_encode(encoded, string, l);
    encoded[l] = '\0'; /* make binary sequence into string */

encoded[l] is out of bounds, because apr_base64_encode() returns the length of
the encoded string, including '\0'. As far as I understand, there is no need to
set '\0' at all, because apr_base64_encode() already appends it. Based on this,
I suggest to get rid of the explicit '\0' setting.

I've attached a patch.

Best Regards,
Denis Kovalchuk
* encoding/apr_base64.c
  (apr_pbase64_encode): Get rid of the explicit '\0' setting for the encoded
   string:
   - encoded[l] is out of bounds, because apr_base64_encode() returns the
     length of the encoded string, including '\0'.
   - apr_base64_encode() already appends '\0'.

Patch by: Denis Kovalchuk <denis.kovalc...@visualsvn.com>

Index: encoding/apr_base64.c
===================================================================
--- encoding/apr_base64.c       (revision 1882695)
+++ encoding/apr_base64.c       (working copy)
@@ -286,8 +286,7 @@ APR_DECLARE(char *) apr_pbase64_encode(apr_pool_t
     int l = strlen(string);
 
     encoded = (char *) apr_palloc(p, apr_base64_encode_len(l));
-    l = apr_base64_encode(encoded, string, l);
-    encoded[l] = '\0'; /* make binary sequence into string */
+    apr_base64_encode(encoded, string, l);
 
     return encoded;
 }

Reply via email to