From 07890fbeb5e65f242e8632ed957c54e188779af2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 23 Aug 2009 22:25:12 +0000 Subject: [PATCH] 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 --- docs/changes.txt | 1 + interface/wx/base64.h | 3 ++- src/common/base64.cpp | 11 +++++++++-- tests/base64/base64.cpp | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 3 deletions(-) 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() -- 2.45.2