Never overflow the output buffer in wxBase64Decode().

Don't write extra NUL bytes obtained by decoding the padding at the end of
input into the output buffer as there may be not enough place in it for them.
And in any case the buffer is not (always) NUL-terminated as no NUL bytes are
obtained in absence of padding, so it's better to never terminate it for
consistency.

Closes #11101.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@61753 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/docs/changes.txt b/docs/changes.txt
index 35b0d00c502fe4b20e2c9f7f8fd1cb89b83fd294..a11a311149f2df751659954de595e5b32d0d1633 100644 (file)
--- a/docs/changes.txt
+++ b/docs/changes.txt
@@ -371,6 +371,7 @@ All:
  * wxLog::DoLogRecord() has access to the location of the log message
    (file, line and function name) and id of the thread which generated it.
  * SetThreadActiveTarget() allows to set up thread-specific log targets.
+- Fix output buffer overflow in wxBase64Decode() (Eric W. Savage).
 
 All (GUI):
 

diff --git a/interface/wx/base64.h b/interface/wx/base64.h
index 02c39d63339d9368c63b17e46d9b2d7c526a1ff8..f72320ee9f936bb640e01ca4e3aace0694083ae1 100644 (file)
--- a/interface/wx/base64.h
+++ b/interface/wx/base64.h
@@ -106,7 +106,8 @@ size_t wxBase64EncodedSize(size_t len);
     This overload is a raw decoding function and decodes the data into the
     provided buffer @a dst of the given size @e dstLen. An error is returned if
     the buffer is not large enough -- that is not at least
-    wxBase64DecodedSize(srcLen) bytes.
+    wxBase64DecodedSize(srcLen) bytes. Notice that the buffer will @e not be
+    @NULL-terminated.
 
     This overload returns the number of bytes written to the buffer or the
     necessary buffer size if @a dst was @NULL or @c wxCONV_FAILED on error,

diff --git a/src/common/base64.cpp b/src/common/base64.cpp
index bf5a613dff15beae9d4b9a622831a3aa248340f1..d83daee68236e4ccc8598377a42c7dfbc20a9c44 100644 (file)
--- a/src/common/base64.cpp
+++ b/src/common/base64.cpp
@@ -187,8 +187,15 @@ wxBase64Decode(void *dst_, size_t dstLen,
 
                 // undo the bit shifting done during encoding
                 *dst++ = in[0] << 2 | in[1] >> 4;
-                *dst++ = in[1] << 4 | in[2] >> 2;
-                *dst++ = in[2] << 6 | in[3];
+
+                // be careful to not overwrite the output buffer with NUL pad
+                // bytes
+                if ( padLen != 2 )
+                {
+                    *dst++ = in[1] << 4 | in[2] >> 2;
+                    if ( !padLen )
+                        *dst++ = in[2] << 6 | in[3];
+                }
             }
 
             n = 0;

diff --git a/tests/base64/base64.cpp b/tests/base64/base64.cpp
index eddc6464716e0d9b459c014203a66026b9f6b95a..85c6e205392812e73c994ac25d7c2ef473a470b9 100644 (file)
--- a/tests/base64/base64.cpp
+++ b/tests/base64/base64.cpp
@@ -142,6 +142,13 @@ void Base64TestCase::EncodeDecodeA()
     wxMemoryBuffer buf = wxBase64Decode(str);
     CPPUNIT_ASSERT_EQUAL(1, buf.GetDataLen());
     CPPUNIT_ASSERT_EQUAL('A', *(char *)buf.GetData());
+
+    char cbuf[10];
+    memset(cbuf, (char)-1, sizeof(cbuf));
+    CPPUNIT_ASSERT_EQUAL( 1, wxBase64Decode(cbuf, 1, str) );
+    CPPUNIT_ASSERT_EQUAL( 'A', cbuf[0] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[1] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[2] );
 }
 
 void Base64TestCase::EncodeDecodeAB()
@@ -153,6 +160,15 @@ void Base64TestCase::EncodeDecodeAB()
     CPPUNIT_ASSERT_EQUAL(2, buf.GetDataLen());
     CPPUNIT_ASSERT_EQUAL('A', buf[0]);
     CPPUNIT_ASSERT_EQUAL('B', buf[1]);
+
+    char cbuf[10];
+    memset(cbuf, (char)-1, sizeof(cbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, wxBase64Decode(cbuf, 1, str) );
+    CPPUNIT_ASSERT_EQUAL( 2, wxBase64Decode(cbuf, 2, str) );
+    CPPUNIT_ASSERT_EQUAL( 'A', cbuf[0] );
+    CPPUNIT_ASSERT_EQUAL( 'B', cbuf[1] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[2] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[3] );
 }
 
 void Base64TestCase::EncodeDecodeABC()
@@ -165,6 +181,16 @@ void Base64TestCase::EncodeDecodeABC()
     CPPUNIT_ASSERT_EQUAL('A', buf[0]);
     CPPUNIT_ASSERT_EQUAL('B', buf[1]);
     CPPUNIT_ASSERT_EQUAL('C', buf[2]);
+
+    char cbuf[10];
+    memset(cbuf, (char)-1, sizeof(cbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, wxBase64Decode(cbuf, 2, str) );
+    CPPUNIT_ASSERT_EQUAL( 3, wxBase64Decode(cbuf, 3, str) );
+    CPPUNIT_ASSERT_EQUAL( 'A', cbuf[0] );
+    CPPUNIT_ASSERT_EQUAL( 'B', cbuf[1] );
+    CPPUNIT_ASSERT_EQUAL( 'C', cbuf[2] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[3] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[4] );
 }
 
 void Base64TestCase::EncodeDecodeABCD()
@@ -178,6 +204,17 @@ void Base64TestCase::EncodeDecodeABCD()
     CPPUNIT_ASSERT_EQUAL('B', buf[1]);
     CPPUNIT_ASSERT_EQUAL('C', buf[2]);
     CPPUNIT_ASSERT_EQUAL('D', buf[3]);
+
+    char cbuf[10];
+    memset(cbuf, (char)-1, sizeof(cbuf));
+    CPPUNIT_ASSERT_EQUAL( wxCONV_FAILED, wxBase64Decode(cbuf, 3, str) );
+    CPPUNIT_ASSERT_EQUAL( 4, wxBase64Decode(cbuf, 4, str) );
+    CPPUNIT_ASSERT_EQUAL( 'A', cbuf[0] );
+    CPPUNIT_ASSERT_EQUAL( 'B', cbuf[1] );
+    CPPUNIT_ASSERT_EQUAL( 'C', cbuf[2] );
+    CPPUNIT_ASSERT_EQUAL( 'D', cbuf[3] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[4] );
+    CPPUNIT_ASSERT_EQUAL( (char)-1, cbuf[5] );
 }
 
 void Base64TestCase::EncodeDecode0to255()