Critical bug: buffer overflow in catch's handling of arguments

[netheril96]

Compiling with address sanitizer that comes with the latest gcc or clang, we can find that the latest catch has buffer overflow when parsing arguments. Here is a screenshot:

Both v1.5.4 and v1.5.3 have the same bug. I have not tested earlier versions.

[netheril96]

Specifically, in Parser::handleOpt, the original code is

if( std::string( ":=\0", 5 ).find( c ) == std::string::npos )

The length of string literal is only 3, yet the constructor reads 5 chars out of it.

Similarly, in Parser::handlePositional,

if( inQuotes || std::string( "\0", 3 ).find( c ) == std::string::npos )

The constructor reads 3 characters out of length-1 string literal.

Both of these cases result in undefined behavior.

[ianmacs]

I agree this is a bug. It was introduced in v1.5.2.

suggest these fixes:
if (c != ':' && c != '=' && c != '\0')
if (inQuotes || c != '\0')

[rioderelfte]

I have already created a pull request in Clara to fix this problem: catchorg/Clara#17

[philsquared]

Ouch! Sorry about that. I'll try and get this sorted (in both code bases) later today - thanks for bringing it up (and @rioderelfte for the PR)

[ianmacs]

Thanks for fixing this in v1.5.6.
This bug can be closed.

[horenmar]

Since this was fixed, I am closing the issue.