Skip to content

Commit

Permalink
Fix concurrency issue in mutlithreaded environment (2.rc.03)
Browse files Browse the repository at this point in the history
Problem:
If two threads called base64_encode concurrently (one with url=true and
the other with url=false, the values of base64_chars[62] and
base64_chars[63] are undefined. For example, it's possible to get
base64_chars[62]='-' and base64_chars[63]='/', which is not a valid
encoding.

This commit fixes this issue.
  • Loading branch information
ReneNyffenegger committed May 13, 2020
1 parent 2ca8b88 commit ac7161c
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 24 deletions.
51 changes: 30 additions & 21 deletions base64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
More information at
https://renenyffenegger.ch/notes/development/Base64/Encoding-and-decoding-base-64-with-cpp
Version: 2.rc.02 (release candidate)
Version: 2.rc.03 (release candidate)
Copyright (C) 2004-2017, 2020 René Nyffenegger
Expand Down Expand Up @@ -33,16 +33,25 @@

#include "base64.h"

static std::string base64_chars =
//
// Depending on the url parameter in base64_chars, one of
// two sets of base64 characters needs to be chosen.
// They differ in their last two characters.
//
const char* base64_chars[2] = {
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
"0123456789"
"??"; // These two question marks will be replaced based on the value of url in base64_encode
"+/",

"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
"0123456789"
"-_"};

static std::size_t pos_of_char(const unsigned char chr) {
//
// Return the position of chr within base64_chars.
// Return the position of chr within base64_encode()
//

if (chr >= 'A' && chr <= 'Z') return chr - 'A';
Expand Down Expand Up @@ -93,45 +102,45 @@ static std::string encode(String s, bool url) {
}

std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len, bool url) {
//
// Replace question marks in base64_chars:
//
if (url) {
base64_chars[62] = '-';
base64_chars[63] = '_';
}
else {
base64_chars[62] = '+';
base64_chars[63] = '/';
}

unsigned int len_encoded = (in_len +2) / 3 * 4;

unsigned char trailing_char = url ? '.' : '=';

//
// Choose set of base64 characters. They differ
// for the last two positions, depending on the url
// parameter.
// A bool (as is the parameter url) is guaranteed
// to evaluate to either 0 or 1 in C++ therfore,
// the correct character set is chosen by subscripting
// base64_chars with url.
//
const char* base64_chars_ = base64_chars[url];

std::string ret;
ret.reserve(len_encoded);

unsigned int pos = 0;

while (pos < in_len) {
ret.push_back(base64_chars[(bytes_to_encode[pos + 0] & 0xfc) >> 2]);
ret.push_back(base64_chars_[(bytes_to_encode[pos + 0] & 0xfc) >> 2]);

if (pos+1 < in_len) {
ret.push_back(base64_chars[((bytes_to_encode[pos + 0] & 0x03) << 4) + ((bytes_to_encode[pos + 1] & 0xf0) >> 4)]);
ret.push_back(base64_chars_[((bytes_to_encode[pos + 0] & 0x03) << 4) + ((bytes_to_encode[pos + 1] & 0xf0) >> 4)]);

if (pos+2 < in_len) {
ret.push_back(base64_chars[((bytes_to_encode[pos + 1] & 0x0f) << 2) + ((bytes_to_encode[pos + 2] & 0xc0) >> 6)]);
ret.push_back(base64_chars[ bytes_to_encode[pos + 2] & 0x3f]);
ret.push_back(base64_chars_[((bytes_to_encode[pos + 1] & 0x0f) << 2) + ((bytes_to_encode[pos + 2] & 0xc0) >> 6)]);
ret.push_back(base64_chars_[ bytes_to_encode[pos + 2] & 0x3f]);
}
else {
ret.push_back(base64_chars[(bytes_to_encode[pos + 1] & 0x0f) << 2]);
ret.push_back(base64_chars_[(bytes_to_encode[pos + 1] & 0x0f) << 2]);
ret.push_back(trailing_char);
}
}
else {

ret.push_back(base64_chars[(bytes_to_encode[pos + 0] & 0x03) << 4]);
ret.push_back(base64_chars_[(bytes_to_encode[pos + 0] & 0x03) << 4]);
ret.push_back(trailing_char);
ret.push_back(trailing_char);
}
Expand Down
2 changes: 1 addition & 1 deletion base64.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//
// base64 encoding and decoding with C++.
// Version: 2.rc.02 (release candidate)
// Version: 2.rc.03 (release candidate)
//

#ifndef BASE64_H_C0CE2A47_D10E_42C9_A27C_C883944E704A
Expand Down
50 changes: 48 additions & 2 deletions test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,58 @@ int main() {
// test checks if this bug was fixed:
//
std::string a17_orig = "aaaaaaaaaaaaaaaaa";
std::string a17_encoded = base64_encode(a17_orig, true);
if (base64_decode(a17_encoded) != a17_orig) {
std::string a17_encoded = base64_encode(a17_orig, false);
std::string a17_encoded_url = base64_encode(a17_orig, true );

if (a17_encoded != "YWFhYWFhYWFhYWFhYWFhYWE=") {
std::cout << "Failed to encode a17" << std::endl;
all_tests_passed = false;
}

if (a17_encoded_url != "YWFhYWFhYWFhYWFhYWFhYWE.") {
std::cout << "Failed to encode a17 (url)" << std::endl;
all_tests_passed = false;
}

if (base64_decode(a17_encoded_url) != a17_orig) {
std::cout << "Failed to decode a17 (url)" << std::endl;
all_tests_passed = false;
}

if (base64_decode(a17_encoded ) != a17_orig) {
std::cout << "Failed to decode a17 (no url)" << std::endl;
all_tests_passed = false;
}

// --------------------------------------------------------------

// characters 63 and 64 / URL encoding

std::string s_6364 = "\x03" "\xef" "\xff" "\xf9";

std::string s_6364_encoded = base64_encode(s_6364, false);
std::string s_6364_encoded_url = base64_encode(s_6364, true );

if (s_6364_encoded != "A+//+Q==") {
std::cout << "Failed to encode_6364 (no url)" << std::endl;
all_tests_passed = false;
}

if (s_6364_encoded_url!= "A-__-Q..") {
std::cout << "Failed to encode_6364 (url)" << std::endl;
all_tests_passed = false;
}

if (base64_decode(s_6364_encoded ) != s_6364) {
std::cout << "Failed to decode s_6364_encoded" << std::endl;
all_tests_passed = false;
}

if (base64_decode(s_6364_encoded_url) != s_6364) {
std::cout << "Failed to decode s_6364_encoded_url" << std::endl;
all_tests_passed = false;
}


// --------------------------------------------------------------

Expand Down

0 comments on commit ac7161c

Please sign in to comment.