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 post_message_to_js memory out of bounds #1114

Merged
merged 4 commits into from
Mar 19, 2024

Conversation

bgrgicak
Copy link
Collaborator

@bgrgicak bgrgicak commented Mar 19, 2024

Fixes #1112

What is this PR doing?

It fixes a bug in post_message_to_js that broke sending messages in Node.

What problem is it solving?

It checks if the response length of a message is larger than -1 before attempting to process the message.

How is the problem addressed?

By checking the response length.

Testing Instructions

  • Confirm that all tests pass

@bgrgicak bgrgicak self-assigned this Mar 19, 2024
@bgrgicak bgrgicak added [Type] Bug An existing feature does not function as intended [Feature] PHP.wasm [Aspect] Node.js labels Mar 19, 2024
@bgrgicak bgrgicak marked this pull request as ready for review March 19, 2024 08:02
@bgrgicak bgrgicak requested a review from adamziel March 19, 2024 08:02
@@ -396,7 +396,7 @@ PHP_FUNCTION(post_message_to_js)

char *response;
size_t response_len = js_module_onMessage(data, &response);
if (response != -1)
if (response_len != -1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

JS returns -1 if there was an error processing the message to response_len. Checking response was the error.

@adamziel adamziel merged commit 2ee9663 into trunk Mar 19, 2024
5 checks passed
@adamziel adamziel deleted the fix/1112-message-to-js-memory-access-out-of-bounds branch March 19, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Aspect] Node.js [Feature] PHP.wasm [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: memory access out of bounds in unit test
2 participants