smuellerDD / acvpparser

ACVP Parser for invocation of cryptographic implementations using the ACVP JSON test vectors
https://www.chronox.de/acvpparser
Other
36 stars 27 forks source link

Sporadic crashes on AES-CTR #88

Closed powersmc closed 1 month ago

powersmc commented 1 month ago

On three instances, I encountered bugs where the parser would crash (SEGFAULT) when attempting to process an AES-CTR vector. That being said, based on my investigation I don't think the issue was unique to AES-CTR. I believe the core issue resides here:

https://github.com/smuellerDD/acvpparser/blob/master/parser/read_json.c#L506

This code uses mmap to access the file contents as a buffer, and then subsequent code passes the buffer to string functions like strlen (https://github.com/smuellerDD/acvpparser/blob/master/parser/json-c/json_tokener.c#L260), which can end up accessing past the end of the buffer, as mmap doesn't guarantee a NULL terminator at the end of the buffer.

I was able to fix this locally by just switching from mmap to malloc, and adding in a NULL terminator myself. I can create a PR if desired, but I wasn't sure if that's the approach you wanted to take to fix this issue, or if you had something else in mind.

smuellerDD commented 1 month ago

Am Montag, 30. September 2024, 21:09:55 MESZ schrieb Mike P:

Hi Mike,

On three instances, I encountered bugs where the parser would crash (SEGFAULT) when attempting to process an AES-CTR vector. That being said, based on my investigation I don't think the issue was unique to AES-CTR. I believe the core issue resides here:

https://github.com/smuellerDD/acvpparser/blob/master/parser/read_json.c#L506

This code uses mmap to access the file contents as a buffer, and then subsequent code passes the buffer to string functions like strlen (https://github.com/smuellerDD/acvpparser/blob/master/parser/json-c/json_to kener.c#L260), which can end up accessing past the end of the buffer, as mmap doesn't guarantee a NULL terminator at the end of the buffer.

I actually removed any malloc as this would increase the memory pressure.

Can you please check the following patch?

diff --git a/parser/read_json.c b/parser/read_json.c index b4143ce84443..f8eaec0fc645 100644 --- a/parser/read_json.c +++ b/parser/read_json.c @@ -478,6 +478,29 @@ static int check_filetype(int fd, struct stat *sb) }

endif

+static struct json_object json_tokener_parse_internal(const char str,

Ciao Stephan

powersmc commented 1 month ago

Hi Stephan,

Yes, that patch does resolve the segfault on my end.

smuellerDD commented 1 month ago

Applied, thanks