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 stack overflow in CVE-2023-31922 #182

Closed

Conversation

nickva
Copy link
Contributor

@nickva nickva commented May 28, 2023

isArray and proxy isArray can call each other indefinitely in a mutually recursive loop.

Add a stack overflow check in the js_proxy_isArray function before calling JS_isArray(ctx, s->target).

With ASAN and the poc.js from issue #178:

./qjs ./poc.js
InternalError: stack overflow
  at isArray (native)
  at <eval> (./poc.js:4)

Fix: #178

isArray and proxy isArray can call each other indefinitely in a mutually
recursive loop.

Add a stack overflow check in the js_proxy_isArray function before calling
JS_isArray(ctx, s->target).

With ASAN the the poc.js from issue 178:

```
./qjs ./poc.js
InternalError: stack overflow
  at isArray (native)
  at <eval> (./poc.js:4)
```

Fix: bellard#178
rweickelt pushed a commit to rweickelt/qbs that referenced this pull request Jun 20, 2023
As published here: bellard/quickjs#182

Change-Id: Icec59a726df1b9015860e23d1f8e603927dd2908
qtprojectorg pushed a commit to qbs/qbs that referenced this pull request Jun 20, 2023
As published here: bellard/quickjs#182

Change-Id: Icec59a726df1b9015860e23d1f8e603927dd2908
Reviewed-by: Ivan Komissarov <[email protected]>
past-due added a commit to Warzone2100/quickjs-wz that referenced this pull request Sep 13, 2023
past-due added a commit to Warzone2100/quickjs-wz that referenced this pull request Sep 13, 2023
@nickva
Copy link
Contributor Author

nickva commented Dec 1, 2023

Noting that this was merged into quickjs-ng quickjs-ng/quickjs#157

@nickva
Copy link
Contributor Author

nickva commented Dec 2, 2023

The patch had been sent to the mailing list https://www.freelists.org/post/quickjs-devel/patch-Stack-overflow-fix-for-CVE202331922 previously and has now been merged to master here as well: 03cc5ec

@nickva
Copy link
Contributor Author

nickva commented Dec 2, 2023

Closing as merged

@nickva nickva closed this Dec 2, 2023
@nickva nickva deleted the fix-cve-2023-31922-stack-overflow branch December 2, 2023 00:33
@tbrockman
Copy link

@nickva thanks for getting this addressed and fixed!

GerHobbelt pushed a commit to GerHobbelt/quickjs that referenced this pull request Sep 14, 2024
…d#182)

Bumps [ws](https://github.com/websockets/ws) from 8.16.0 to 8.17.1.
- [Release notes](https://github.com/websockets/ws/releases)
- [Commits](websockets/ws@8.16.0...8.17.1)

---
updated-dependencies:
- dependency-name: ws
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

AddressSanitizer: stack-overflow
2 participants