Skip to content

Fixed regexp undefined value of captured group. #900

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

Merged
merged 1 commit into from
May 1, 2025

Conversation

VadimZhestikov
Copy link
Contributor

@VadimZhestikov VadimZhestikov commented Apr 30, 2025

The issue was found by OSS-Fuzz. Segmentation fault at the input:

' a'.split(/|()/)

Actually that is old issue: previously (before removing short_strings in #848) to obtain value of captured group was used njs_string_prop() without check for its value argument type and it by mistake converted undefined value to empty string, while there is expected undefined value itself. After removing short_strings, njs_string prop() throws segmentation fault.

@VadimZhestikov VadimZhestikov requested a review from xeioex April 30, 2025 21:58
xeioex
xeioex previously requested changes May 1, 2025
Copy link
Contributor

@xeioex xeioex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For commit log, please, also add

Found by OSS-Fuzz.

@VadimZhestikov VadimZhestikov force-pushed the regexp_captured_group branch from bfce55b to 95e784e Compare May 1, 2025 02:05
@VadimZhestikov VadimZhestikov requested a review from xeioex May 1, 2025 02:07
@VadimZhestikov VadimZhestikov dismissed xeioex’s stale review May 1, 2025 02:08

applied suggested changes

@xeioex xeioex force-pushed the regexp_captured_group branch from 95e784e to 91a8dd1 Compare May 1, 2025 02:12
@VadimZhestikov VadimZhestikov force-pushed the regexp_captured_group branch from 91a8dd1 to 018c6d0 Compare May 1, 2025 02:13
@VadimZhestikov
Copy link
Contributor Author

test262 is ok

@VadimZhestikov VadimZhestikov merged commit 30b5331 into nginx:master May 1, 2025
1 check passed
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.

2 participants