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

Memory corruption fix (bad flood decode bounds check) 🙃 #89

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

sz3
Copy link
Owner

@sz3 sz3 commented Feb 8, 2024

Thanks to @notune for the discovery/stacktrace/fix! (the trifecta!)

Ported upstream from the cfc PR (sz3/cfc#29). I also fixed the logic one layer up (in FloodDecodePositions), so now we have some defense in depth...

sz3/cfc#29
sz3/cfc#28

sz3 and others added 2 commits February 7, 2024 20:35
d'oh

This is functionally identical in all *happy path* cases (index 0 was
being falsely excluded, but it's one of the "seed" positions so it's
already been decoded), but making the completely wrong sanity check and
sending us into oob, undefined behavior territory.
@sz3 sz3 merged commit e77b598 into master Feb 8, 2024
8 checks passed
@sz3 sz3 deleted the bugfix-flood-decode-bounds-check branch February 8, 2024 05:05
sz3 added a commit to sz3/cfc that referenced this pull request Feb 9, 2024
Off by one, memory corruption, we got it all...

#29
sz3/libcimbar#89
sz3/libcimbar#90

Thanks to @notune for the AdjacentCellFinder fix!

Co-authored-by: Noah<noahmuehl@me.com>
sz3 added a commit that referenced this pull request Mar 13, 2024
368c2c4 Merge pull request #90 from sz3/bugfix-santitize-future
258991b Fix some longstanding (no)-fsanitize sloppiness
e77b598 Merge pull request #89 from sz3/bugfix-flood-decode-bounds-check
ead73ea fix memory corruption bug
0fe367f Fix the error check in FloodDecodePositions
97f4af3 Merge pull request #87 from sz3/build-and-css-bugfix-24-2
0f2d718 Tweak to make package-portable-linux slightly less docker dependent
d2ddcc2 Use innerWidth to avert android firefox shenanigans
8f9caa1 If negative z-index is bad vibes, we should set z-index+1 various places
38b8068 Update index.html
0119374 Merge pull request #85 from sz3/pin-cmake-portable-build
452eb71 Pin "portable" linux script to use an old (known good) cmake
05d5e38 Merge pull request #82 from sz3/ios-css-bugfix
69a7d41 Fix bug/change in ios (un)focus behavior?

git-subtree-dir: app/src/cpp/libcimbar
git-subtree-split: 368c2c4
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