-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
growable_comment_array_add leaks, causes crash #84201
Comments
growable_comment_array_add in parsetok.c incorrectly uses realloc, which leaks the array when allocation fails, and then causes a null pointer deref crash later when the array is freed in growable_comment_array_deallocate (the array pointer is dereferenced, passing null to free is fine). It's unlikely that this codepath is reached in normal use, since type comments need to be turned on (via the PyCF_TYPE_COMMENTS compiler flag), but I've managed to replicate the issue by injecting faults with Application Verifier. It's easiest to cause it to fail with a very large number of type comments, but presumably this could also happen with some form of heap fragmentation. The buggy code is: static int
growable_comment_array_add(growable_comment_array *arr, int lineno, char *comment) {
if (arr->num_items >= arr->size) {
arr->size *= 2;
arr->items = realloc(arr->items, arr->size * sizeof(*arr->items));
if (!arr->items) {
return 0;
}
}
arr->items[arr->num_items].lineno = lineno;
arr->items[arr->num_items].comment = comment;
arr->num_items++;
return 1;
} and the correct code would be something like: static int
growable_comment_array_add(growable_comment_array *arr, int lineno, char *comment) {
if (arr->num_items >= arr->size) {
arr->size *= 2;
void* new_items_array = realloc(arr->items, arr->size * sizeof(*arr->items));
if (!new_items_array) {
return 0;
}
arr->items = new_items_array;
}
arr->items[arr->num_items].lineno = lineno;
arr->items[arr->num_items].comment = comment;
arr->num_items++;
return 1;
} |
Sidenote: visual studio was misleading and made this look like a use-after-free for a little while, which was interesting. |
Alexander Riccio: Do you know want to propose a change to replace direct usage of malloc/realloc/free with PyMem_Malloc, PyMem_Realloc and PyMem_RawFree? It would add their builtin debug feature for free, and also detect most obvious buffer overflow (reject size larger than PY_SSIZE_T_MAX). |
Sure, should I open a new issue? |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: