-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sporadic crashes on AES-CTR #88
Comments
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,
+ off_t len)
+{
+ struct json_tokener* tok;
+ struct json_object* obj;
+
+ if (len > INT_MAX || len < 0)
+ return NULL;
+
+ tok = json_tokener_new();
+ if (!tok)
+ return NULL;
+ obj = json_tokener_parse_ex(tok, str, (int)len);
+ if(tok->err != json_tokener_success) {
+ if (obj != NULL)
+ json_object_put(obj);
+ obj = NULL;
+ }
+
+ json_tokener_free(tok);
+ return obj;
+}
+
int json_read_data(const char *filename, struct json_object **inobj)
{
struct json_object *o;
@@ -509,7 +532,7 @@ int json_read_data(const char *filename, struct
json_object **inobj)
close(fd);
return ret;
}
- o = json_tokener_parse(data);
+ o = json_tokener_parse_internal(data, sb.st_size);
munmap(data, (size_t)sb.st_size);
close(fd);
#endif
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.
Ciao
Stephan
|
Hi Stephan, Yes, that patch does resolve the segfault on my end. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The text was updated successfully, but these errors were encountered: