0
0
mirror of https://github.com/OpenVPN/openvpn3.git synced 2024-09-20 12:12:15 +02:00

Base64: fixed issue where decode() method would ignore bad chars rather than throwing base64_decode_error()

Added a unit test to confirm the fix.

Other changes:

* In Base64 decode(), avoid the use of std::strlen() in favor
  of std::string length() method since a std::string could
  conceivably contain embedded null chars.

* In Base64 unit test, renamed b64_test_bad() to
  b64_test_bad_decode() for clarity.

Signed-off-by: James Yonan <james@openvpn.net>
This commit is contained in:
James Yonan 2020-03-08 17:39:03 -06:00
parent 995b6bf467
commit f669831556
3 changed files with 19 additions and 14 deletions

View File

@ -25,7 +25,9 @@
#define OPENVPN_COMMON_BASE64_H
#include <string>
#include <cstring> // for std::memset, std::strlen
#include <cstring> // for std::memset, std::strlen
#include <algorithm> // for std::min
#include <cstddef> // for ptrdiff_t
#include <openvpn/common/size.hpp>
#include <openvpn/common/exception.hpp>
@ -195,10 +197,11 @@ namespace openvpn {
template <typename V>
void decode(V& dest, const std::string& str) const
{
for (const char *p = str.c_str(); *p != '\0' && (*p == equal || is_base64_char(*p)); p += 4)
const char *endp = str.c_str() + str.length();
for (const char *p = str.c_str(); p < endp; p += 4)
{
unsigned int marker;
const unsigned int val = token_decode(p, marker);
const unsigned int val = token_decode(p, std::min(endp - p, ptrdiff_t(4)), marker);
dest.push_back((val >> 16) & 0xff);
if (marker < 2)
dest.push_back((val >> 8) & 0xff);
@ -249,12 +252,12 @@ namespace openvpn {
return v;
}
unsigned int token_decode(const char *token, unsigned int& marker) const
unsigned int token_decode(const char *token, const ptrdiff_t len, unsigned int& marker) const
{
size_t i;
unsigned int val = 0;
marker = 0; // number of equal chars seen
if (std::strlen(token) < 4)
if (len < 4)
throw base64_decode_error();
for (i = 0; i < 4; i++)
{

1
test/unittests/.gitignore vendored Normal file
View File

@ -0,0 +1 @@
test_b64

View File

@ -117,22 +117,23 @@ TEST(Base64, tooshortdest)
EXPECT_THROW(b64.decode(buf, 2, enc), Base64::base64_decode_out_of_bound_error);
}
void b64_test_bad(const Base64& b64, const std::string& text)
void b64_test_bad_decode(const Base64& b64, const std::string& text)
{
std::string dec;
EXPECT_THROW(b64.decode(dec, text), Base64::base64_decode_error);
}
TEST(Base64, badencode)
TEST(Base64, baddecode)
{
const Base64 b64;
b64_test_bad(b64, "plausible deniability");
b64_test_bad(b64, "plausible != deniability");
b64_test_bad(b64, "x");
b64_test_bad(b64, "====");
b64_test_bad(b64, "xxxx=");
b64_test_bad(b64, "01*=");
b64_test_bad_decode(b64, "!@#$%^&*()_");
b64_test_bad_decode(b64, "plausible deniability");
b64_test_bad_decode(b64, "plausible != deniability");
b64_test_bad_decode(b64, "x");
b64_test_bad_decode(b64, "====");
b64_test_bad_decode(b64, "xxxx=");
b64_test_bad_decode(b64, "01*=");
}
TEST(Base64, encode)
@ -178,4 +179,4 @@ TEST(Base64, binary_data)
b64_test_binary(b64, data, i);
delete[] data;
}
}
}