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

tty_playback: Fix handlePlaybackAction panic on unit16 overflow #36409

Merged
merged 2 commits into from
Jan 8, 2024

Conversation

jentfoo
Copy link
Contributor

@jentfoo jentfoo commented Jan 8, 2024

Panic fix followup from #36168

Because the addition was happening prior to the int cast it was possible to bypass the length check and result in a panic.

In addition this case and others were added as fuzz seeds, and enabled in oss-fuzz.

Because the addition was happening prior to the cast it was possible to bypass the length check and result in a panic.
@jentfoo jentfoo requested review from Tener, avatus, zmb3 and rudream January 8, 2024 17:19
@jentfoo jentfoo self-assigned this Jan 8, 2024
@github-actions github-actions bot requested review from klizhentas and r0mant January 8, 2024 17:20
Copy link

github-actions bot commented Jan 8, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@jentfoo jentfoo force-pushed the jent/stream_recording_improvements branch from 00adb8f to 7ff8316 Compare January 8, 2024 17:20
@jentfoo jentfoo added the no-changelog Indicates that a PR does not require a changelog entry label Jan 8, 2024
Comment on lines -29 to +36
handlePlaybackAction(b, player)
_ = handlePlaybackAction(b, player)
Copy link
Contributor

Choose a reason for hiding this comment

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

what does assigning this to nothing do in comparison to just invoking the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing functionally, but it removes a warning in my IDE. I feel it's cleaner to show when you're explicitly ignoring errors

Copy link
Contributor

Choose a reason for hiding this comment

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

that sounds great! Thanks for the info 👍

@jentfoo jentfoo added this pull request to the merge queue Jan 8, 2024
Merged via the queue into master with commit 0510fe1 Jan 8, 2024
35 of 36 checks passed
@jentfoo jentfoo deleted the jent/stream_recording_improvements branch January 8, 2024 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Indicates that a PR does not require a changelog entry size/sm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants