Possible out of range input buffer access

[cyb70289]

Thanks @ReneNyffenegger for this useful c++ base64 library.

I noticed a similar old issue https://github.com/ReneNyffenegger/cpp-base64/issues/24 Looks the problem is not fixed for invalid inputs.

As a simple example, base64_decode(std::string("a")) will access encoded_string[1] at line https://github.com/ReneNyffenegger/cpp-base64/blob/master/base64.cpp#L207, which is beyond the encoded_string size. Current code happen to work as there's a padding \0 after the string. But we cannot depend on that.

If we replace encoded_string[pos+1] with encoded_string.at(pos+1), the test will throw.

terminate called after throwing an instance of 'std::out_of_range'
  what():  basic_string::at: __n (which is 1) >= this->size() (which is 1)

[Satancito]

"a" isn't valid base64 string to decode.

[cyb70289]

"a" isn't valid base64 string to decode.

Indeed. But IMO invalid input should not cause undefined behaviour to a lib.

[pke]

I have to agree with @cyb70289 here. A lib should never trust user input. It should validate everything and all the time. If "a" isn't a valid base64 string to decode then the lib has to properly throw and not just crash (and introduce potential security risks by doing this).

The test suite for this lib would have to be brought up to another level to reveal all those security flaws properly. Starting by adding a regression test for this bug. Irrc the google-test library could also detect invalid memory access, but I would have to check the docs again.

[BullyWiiPlaza]

Agreed, why not replace *all* direct array accesses with calls to at(): Saying this is "slow" is really counter-productive and hides severe bugs such as out of bounds accesses that should not go unnoticed. Also, modern compilers might remove redundant bounds checks anyway. In literally every other language these types of bugs would throw an exception but in C/C++ we prefer to be oblivious? I hope this change will be applied at least @ReneNyffenegger

[ReneNyffenegger]

I've replaced encoded_string[] with encoded_string.at() in decode() (https://github.com/ReneNyffenegger/cpp-base64/commit/38c63155e6995bf17cce30d29203002b0c3ae91a)