Fix CVE-2024-21526 by adding arg checks in binding.open #186
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix CVE-2024-21526 by adding arg checks in binding.open and removing asserts
Re: GHSA-w5fc-gj3h-26rx
From the link:
The changes should prevent the process crashed vuln noted in the CVE.
Callback Info Check: Replaced
assert(napi_get_cb_info...)with a check fornapi_okand verified thatargc >= 4. If this fails, an error is thrown andNULLis returned.Memory Allocation: Added a null check for
mallocofSpeaker. If allocation fails, a memory error is thrown.Channels, Rate, Format: Replaced
assertcalls with checks fornapi_get_value_int32. Added input validation (e.g.,channels <= 0orrate <= 0are considered invalid). On failure, thespeakeris freed and an error is thrown.Device String Handling: Retained the
is_stringcheck. Replacedassertfornapi_get_value_string_utf8with proper error handling. Added memory allocation checks forspeaker->device, and ensured bothspeakeranddeviceare freed on failure.Handle Creation: Combined checks for
napi_create_objectandnapi_wrap. If either fails, resources are cleaned up and an error is thrown.Cleanup: Ensured all error paths properly free allocated memory (
speaker->deviceandspeaker) to prevent memory leaks.Defensive Vibes: Added thorough validation at every step. Invalid inputs or failures result in clear error messages to avoid crashes or undefined behavior.
JavaScript checks: Also added a JS check for .channels before calling
binding.openas another layer.The fork this comes from also updates dependencies (@dosyago-sec/node-speaker - so it can be used in BrowserBox - remote browser isolation and text client for the web), but this is omitted here to avoid affecting existing tests and to minimize changes to the original repo for this PR.