Skip to content
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

fix: null array indexing in CTracer_handle_return #1843

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

frigus02
Copy link
Contributor

@frigus02 frigus02 commented Sep 2, 2024

Fixes #1835

CTracer_set_pdata_stack can initialize self.pdata_stack to an empty stack where ->stack is NULL and ->deph is -1.

Move index into ->stack into ->depth>=0 check to avoid indexing into NULL array.

This issue was found running UndefinedBehaviourSanitizer. It's reproducible in regular test runs, e.g. python3 -m tox -e py311. Adding if (self->pdata_stack->stack == NULL) { fprint(...) } before the moved line shows the issue in several test cases.


The contributing guide mentions ever fix should include tests. I'd be happy to add some, but I'm not quite sure how to do that here. Do you have a preference/advice?

CTracer_set_pdata_stack can initialize self.pdata_stack to an empty
stack where ->stack is NULL and ->deph is -1.

Move index into ->stack into ->depth>=0 check to avoid indexing into
NULL array.

This issue was found running UndefinedBehaviourSanitizer. It's
reproducible in regular test runs, e.g. `python3 -m tox -e py311`.
Adding `if (self->pdata_stack->stack == NULL) { fprint(...) }` before
the moved line shows the issue in several test cases.
@nedbat nedbat merged commit f4c1f01 into nedbat:master Sep 3, 2024
35 checks passed
nedbat added a commit that referenced this pull request Sep 3, 2024
@frigus02 frigus02 deleted the fix-ubsan-issue branch September 3, 2024 07:17
@nedbat
Copy link
Owner

nedbat commented Oct 9, 2024

This is now released as part of coverage 7.6.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

null array indexing in coverage/ctracer/tracer.c
2 participants