diff --git a/docs/changes.txt b/docs/changes.txt index 35b0d00c50..a11a311149 100644 --- 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 02c39d6333..f72320ee9f 100644 --- 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 bf5a613dff..d83daee682 100644 --- 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 eddc646471..85c6e20539 100644 --- 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()