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

3 files changed, 39 insertions, 6 deletions

diff --git a/lib/libc/include/plbase64.h b/lib/libc/include/plbase64.h
index ac07e837..d21ff5d7 100644
--- a/lib/libc/include/plbase64.h
+++ b/lib/libc/include/plbase64.h
@@ -57,6 +57,10 @@ PR_BEGIN_EXTERN_C
  * 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 *)
@@ -83,6 +87,12 @@ PL_Base64Encode
  * 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 *)
diff --git a/lib/libc/src/base64.c b/lib/libc/src/base64.c
index 234d60d8..e3fd80d7 100644
--- a/lib/libc/src/base64.c
+++ b/lib/libc/src/base64.c
@@ -38,7 +38,8 @@
 #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+/";
 
@@ -150,12 +151,24 @@ PL_Base64Encode
 {
     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 )
         {
@@ -383,7 +396,13 @@ PL_Base64Decode
 
     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)) )
@@ -403,7 +422,8 @@ PL_Base64Decode
 
     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 )
         {
diff --git a/lib/libc/src/strlen.c b/lib/libc/src/strlen.c
index 6c476777..e03f7d25 100644
--- a/lib/libc/src/strlen.c
+++ b/lib/libc/src/strlen.c
@@ -53,7 +53,10 @@ PL_strlen(const char *str)
      * 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;
 }