Bug 492779: In PL_Base64Encode and PL_Base64Decode, ensure that all NSPR_4_7_BRANCH
authorwtc%google.com
Sat, 13 Jun 2009 13:41:42 +0000
branchNSPR_4_7_BRANCH
changeset 4130 83ee1d848073950e55c22149e98fc9b7d95634b8
parent 4129 73041caade4f51f5f3fe68c8b5457f6d64584ab1
child 4131 e3a49ba25440738763a0e3dcc521d22c5c15007c
push idunknown
push userunknown
push dateunknown
bugs492779
Bug 492779: In PL_Base64Encode and PL_Base64Decode, ensure that all PRUint32 values stay within range. Call strlen instead of PL_strlen so we can detect size_t to PRUint32 truncations. Added comments to plbase64.h to explain how to avoid PRUint32 overflow when calculating destination buffer sizes. Assert that PL_strlen's return value doesn't overflow PRInt32 in optimized builds as well. r=nelson. Modified Files: plbase64.h base64.c strlen.c Tag: NSPR_4_7_BRANCH
lib/libc/include/plbase64.h
lib/libc/src/base64.c
lib/libc/src/strlen.c
--- a/lib/libc/include/plbase64.h
+++ b/lib/libc/include/plbase64.h
@@ -52,16 +52,20 @@ PR_BEGIN_EXTERN_C
  * is used to determine the source length.  If the "dest" parameter is not
  * null, it is assumed to point to a buffer of sufficient size (which may be
  * calculated: ((srclen + 2)/3)*4) into which the encoded data is placed 
  * (without any termination).  If the "dest" parameter is null, a buffer is
  * allocated from the heap to hold the encoded data, and the result *will*
  * be terminated with an extra null character.  It is the caller's 
  * responsibility to free the result when it is allocated.  A null is returned 
  * if the allocation fails.
+ *
+ * NOTE: when calculating ((srclen + 2)/3)*4), first ensure that
+ *     srclen <= (PR_UINT32_MAX/4) * 3
+ * to avoid PRUint32 overflow.
  */
 
 PR_EXTERN(char *)
 PL_Base64Encode
 (
     const char *src,
     PRUint32    srclen,
     char       *dest
@@ -78,16 +82,22 @@ PL_Base64Encode
  * length.  If the "dest" parameter is not null, it is assumed to point to
  * a buffer of sufficient size (which may be calculated: (srclen * 3)/4
  * when srclen includes the '=' characters) into which the decoded data
  * is placed (without any termination).  If the "dest" parameter is null,
  * a buffer is allocated from the heap to hold the decoded data, and the
  * result *will* be terminated with an extra null character.  It is the
  * caller's responsibility to free the result when it is allocated.  A null
  * is retuned if the allocation fails, or if the source is not well-coded.
+ *
+ * NOTE: when calculating (srclen * 3)/4, first ensure that 
+ *     srclen <= PR_UINT32_MAX/3
+ * to avoid PRUint32 overflow.  Alternatively, calculate
+ *     (srclen/4) * 3 + ((srclen%4) * 3)/4
+ * which is equivalent but doesn't overflow for any value of srclen.
  */
 
 PR_EXTERN(char *)
 PL_Base64Decode
 (
     const char *src,
     PRUint32    srclen,
     char       *dest
--- a/lib/libc/src/base64.c
+++ b/lib/libc/src/base64.c
@@ -33,17 +33,18 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "plbase64.h"
 #include "prlog.h" /* For PR_NOT_REACHED */
 #include "prmem.h" /* for malloc / PR_MALLOC */
-#include "plstr.h" /* for PL_strlen */
+
+#include <string.h> /* for strlen */
 
 static unsigned char *base = (unsigned char *)"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
 
 static void
 encode3to4
 (
     const unsigned char    *src,
     unsigned char          *dest
@@ -145,22 +146,34 @@ PL_Base64Encode
 (
     const char *src,
     PRUint32    srclen,
     char       *dest
 )
 {
     if( 0 == srclen )
     {
-        srclen = PL_strlen(src);
+        size_t len = strlen(src);
+        srclen = len;
+        /* Detect truncation. */
+        if( srclen != len )
+        {
+            return (char *)0;
+        }
     }
 
     if( (char *)0 == dest )
     {
-        PRUint32 destlen = ((srclen + 2)/3) * 4;
+        PRUint32 destlen;
+        /* Ensure all PRUint32 values stay within range. */
+        if( srclen > (PR_UINT32_MAX/4) * 3 )
+        {
+            return (char *)0;
+        }
+        destlen = ((srclen + 2)/3) * 4;
         dest = (char *)PR_MALLOC(destlen + 1);
         if( (char *)0 == dest )
         {
             return (char *)0;
         }
         dest[ destlen ] = (char)0; /* null terminate */
     }
 
@@ -378,17 +391,23 @@ PL_Base64Decode
 
     if( (char *)0 == src )
     {
         return (char *)0;
     }
 
     if( 0 == srclen )
     {
-        srclen = PL_strlen(src);
+        size_t len = strlen(src);
+        srclen = len;
+        /* Detect truncation. */
+        if( srclen != len )
+        {
+            return (char *)0;
+        }
     }
 
     if( srclen && (0 == (srclen & 3)) )
     {
         if( (char)'=' == src[ srclen-1 ] )
         {
             if( (char)'=' == src[ srclen-2 ] )
             {
@@ -398,17 +417,18 @@ PL_Base64Decode
             {
                 srclen -= 1;
             }
         }
     }
 
     if( (char *)0 == dest )
     {
-        PRUint32 destlen = ((srclen * 3) / 4);
+        /* The following computes ((srclen * 3) / 4) without overflow. */
+        PRUint32 destlen = (srclen / 4) * 3 + ((srclen % 4) * 3) / 4;
         dest = (char *)PR_MALLOC(destlen + 1);
         if( (char *)0 == dest )
         {
             return (char *)0;
         }
         dest[ destlen ] = (char)0; /* null terminate */
         allocated = PR_TRUE;
     }
--- a/lib/libc/src/strlen.c
+++ b/lib/libc/src/strlen.c
@@ -48,17 +48,20 @@ PL_strlen(const char *str)
     if( (const char *)0 == str ) return 0;
 
     l = strlen(str);
 
     /* error checking in case we have a 64-bit platform -- make sure
      * we don't have ultra long strings that overflow an int32
      */ 
     if( sizeof(PRUint32) < sizeof(size_t) )
-        PR_ASSERT(l < 2147483647);
+    {
+        if( l > PR_INT32_MAX )
+            PR_Assert("l <= PR_INT32_MAX", __FILE__, __LINE__);
+    }
 
     return (PRUint32)l;
 }
 
 PR_IMPLEMENT(PRUint32)
 PL_strnlen(const char *str, PRUint32 max)
 {
     register const char *s;