Skip to content

Add index bounds checks to negentropy ingester#219

Merged
hoytech merged 1 commit intohoytech:masterfrom
theAnuragMishra:fix-negentropy-ingester
Apr 28, 2026
Merged

Add index bounds checks to negentropy ingester#219
hoytech merged 1 commit intohoytech:masterfrom
theAnuragMishra:fix-negentropy-ingester

Conversation

@theAnuragMishra
Copy link
Copy Markdown
Contributor

In the current state, it just trusts the inputarr.
This pr adds a check to verify the length of the input before tryiing to access arr.at(0) to check the type of the command.

Additionally, there might be some performance benefits of doing arr.get_array() beforehand and then direct index access instead of doing arr.at(i). Not sure how significant though.

@hoytech
Copy link
Copy Markdown
Owner

hoytech commented Apr 28, 2026

The size of the array is verified to be at least 2 in the runIngester loop (same with all commands), so the first arr[1] is safe. The NEG-OPEN branch then verifies at least 3 elements. And NEG-MSG uses .at() to get the message (which is bound checked).

So I don't think there actually is a missing bounds check anywhere.

However, your PR does clean up the code, and will provide a better error message in case of a malformed NEG-MSG command. The redundant verification of vals.size() is no big deal in practice, so I will merge. Thank you!

@hoytech hoytech merged commit fc64e69 into hoytech:master Apr 28, 2026
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