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

internal/fuzz: int32 corpus values that aren't valid UTF-8 runes marshal as 0xFFFD #51528

Closed
bcmills opened this issue Mar 7, 2022 · 4 comments
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Mar 7, 2022

I found #51526 by examining the edge-cases in the internal/fuzz encoding tests, and it also affects internal/fuzz.

int32 values such as 0xd800 (half a surrogate pair) and unicode.MaxRune + 1 marshaled to the Unicode replacement character (0xFFFD) instead of the original character, losing both reproducibility and input-diversity in the fuzz corpus.

@bcmills bcmills added fuzz Issues related to native fuzzing support release-blocker labels Mar 7, 2022
@bcmills bcmills added this to the Go1.18 milestone Mar 7, 2022
@bcmills bcmills self-assigned this Mar 7, 2022
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 7, 2022
@gopherbot
Copy link

Change https://go.dev/cl/390424 mentions this issue: internal/fuzz: fix encoding for out-of-range ints and runes

@bcmills
Copy link
Contributor Author

bcmills commented Mar 8, 2022

Reopening for 1.18 backport.

@gopherbot
Copy link

Change https://go.dev/cl/390816 mentions this issue: [release-branch.go1.18] internal/fuzz: fix encoding for out-of-range ints and runes

@dmitshur
Copy link
Contributor

dmitshur commented Mar 9, 2022

Closed by merging commit bf366ef (CL 390816) to release-branch.go1.18.

@dmitshur dmitshur closed this as completed Mar 9, 2022
gopherbot pushed a commit that referenced this issue Mar 9, 2022
…ints and runes

Also switch float64 NaN encoding to use hexadecimal, and accept
hexadecimal encoding for all other integer types too. (That gives us
the flexibility to change the encodings in either direction in the
future without breaking earlier Go versions.)

Out-of-range runes encoded using "%q" were previously replaced with
the Unicode replacement charecter, losing their values.

Out-of-range ints and uints on 32-bit platforms were previously
rejected. Now they are wrapped instead: an “interesting” case with a
large int or uint found on a 64-bit platform likely remains
interesting on a 32-bit platform, even if the specific values differ.

To verify the above changes, I have made TestMarshalUnmarshal accept
(and check for) arbitrary differences between input and output, and
added tests cases that include values in valid but non-canonical
encodings.

I have also added round-trip fuzz tests in the opposite direction for
most of the types affected by this change, verifying that a marshaled
value unmarshals to the same bitwise value.

Updates #51258
Updates #51526
Fixes #51528

Change-Id: I7727a9d0582d81be0d954529545678a4374e88ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/390424
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 7419bb3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/390816
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
@rsc rsc unassigned bcmills Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge fuzz Issues related to native fuzzing support NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
Status: No status
Development

No branches or pull requests

3 participants