Skip to content

Fix UB: use || instead of && #7779

Merged
Manishearth merged 2 commits intounicode-org:mainfrom
sayrer:fix/resb-alignment-check
Mar 15, 2026
Merged

Fix UB: use || instead of && #7779
Manishearth merged 2 commits intounicode-org:mainfrom
sayrer:fix/resb-alignment-check

Conversation

@sayrer
Copy link
Contributor

@sayrer sayrer commented Mar 14, 2026

in cast_bytes_to_slice alignment/length check

The condition guarding from_raw_parts joined the alignment check and length check with &&, meaning the error was only returned when both were wrong. A misaligned-but-correctly-sized buffer passed the check, then from_raw_parts dereferences misaligned data — undefined behavior.

Added tests that verify each condition is rejected independently.

Changelog

resb: Fix UB around alignment check

…check

The condition guarding `from_raw_parts` joined the alignment check and
length check with `&&`, meaning the error was only returned when *both*
were wrong. A misaligned-but-correctly-sized buffer passed the check,
then `from_raw_parts` dereferences misaligned data — undefined behavior.

Added tests that verify each condition is rejected independently.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sayrer sayrer requested a review from a team as a code owner March 14, 2026 22:49
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks! I'm pretty sure the tests can be written in a way that doesn't need unsafe but I'll look into that later.

@Manishearth Manishearth enabled auto-merge (squash) March 14, 2026 23:59
@Manishearth
Copy link
Member

Please run cargo fmt

@Manishearth
Copy link
Member

Oh and you'll have to sign the CLA unfortunately.

@sayrer
Copy link
Contributor Author

sayrer commented Mar 15, 2026

Oh, I don't care about the CLA. It's 6pm here, I'll figure it out tomorrow.

@Manishearth
Copy link
Member

Yeah you just have to go click a button on link on the failing ci task.

auto-merge was automatically disabled March 15, 2026 16:29

Head branch was pushed to by a user without write access

@CLAassistant
Copy link

CLAassistant commented Mar 15, 2026

CLA assistant check
All committers have signed the CLA.

@sayrer
Copy link
Contributor Author

sayrer commented Mar 15, 2026

Nice catch, thanks! I'm pretty sure the tests can be written in a way that doesn't need unsafe but I'll look into that later.

Maybe so, I never use it, so my knowledge is just from FFI wrappers and the Rust Book.

@Manishearth Manishearth enabled auto-merge (squash) March 15, 2026 17:27
@Manishearth Manishearth merged commit 9c0923d into unicode-org:main Mar 15, 2026
34 checks 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.

3 participants