[text] secure/precis: fix short destination buffer handling in Nickname profile
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil added 3 comments![]( "Open in Gerrit")
Fixes CVE-2026-56851Reference public issue (file public issue if none exists yet).
if nDst+need > len(dst) {This changes the function behavior when dst is too short: Previously, it would fill as much of dst as possible, while now it may fill only part of dst.
I don't know if this matters, but it's simple to avoid changing it so let's keep things as stable as possible: When assigning `dst[nDst] = ' '`, we can return an error if `nDst >= len(dst)`.
func TestTransform(t *testing.T) {There's a `TestTransformShortBuffers` in enforce_test.go. Looks like it currently only uses a profile that doesn't exercise the Nickname case. Can we make this run with various profiles (and possibly inputs) to exercise this fix?
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 3 comments![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
Reference public issue (file public issue if none exists yet).
Done
This changes the function behavior when dst is too short: Previously, it would fill as much of dst as possible, while now it may fill only part of dst.
I don't know if this matters, but it's simple to avoid changing it so let's keep things as stable as possible: When assigning `dst[nDst] = ' '`, we can return an error if `nDst >= len(dst)`.
Done
There's a `TestTransformShortBuffers` in enforce_test.go. Looks like it currently only uses a profile that doesn't exercise the Nickname case. Can we make this run with various profiles (and possibly inputs) to exercise this fix?
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil added 5 comments![]( "Open in Gerrit")
Full disclosure: Gemini found all of this.
if size == 0 { // Incomplete UTF-8 encodingPreexisting problem: size is never 0, `DecodeRune` returns `(RuneError, 1)` when encountering a partial rune.
This results in, for example, `"a\xef\xb7"` (an input ending in an incomplete 3-byte rune) returning `"a\xef\xb7"` rather than `"a"` and ErrShortSrc.
return nDst, nSrc, transform.ErrShortDstThis leaves t.prevSpace set, but the space has already been appended to the destination.
For example, `"a b"` with a 2-byte destination produces `"a "`, and the next call produces `" b"` rather than `"b"`.
return nDst, nSrc, transform.ErrShortDstThis can write part of a multi-byte rune to the dst without advancing nSrc.
For example, `"\uFDFA"` (a 3-byte rune) with a 2-byte destination produces nSrc=0, nDst=2, and `"\xef\xb7"`. It should produce nSrc=0, nDst=0.
return nDst, nSrc, transform.ErrShortSrcMost transformers return a nil error when `nSrc == len(src) && !atEOF` (all source bytes consumed and written to dst, no dangling state).
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 4 comments![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
4 comments
Preexisting problem: size is never 0, `DecodeRune` returns `(RuneError, 1)` when encountering a partial rune.
This results in, for example, `"a\xef\xb7"` (an input ending in an incomplete 3-byte rune) returning `"a\xef\xb7"` rather than `"a"` and ErrShortSrc.
Done
This leaves t.prevSpace set, but the space has already been appended to the destination.
For example, `"a b"` with a 2-byte destination produces `"a "`, and the next call produces `" b"` rather than `"b"`.
Done
This can write part of a multi-byte rune to the dst without advancing nSrc.
For example, `"\uFDFA"` (a 3-byte rune) with a 2-byte destination produces nSrc=0, nDst=2, and `"\xef\xb7"`. It should produce nSrc=0, nDst=0.
Done
Most transformers return a nil error when `nSrc == len(src) && !atEOF` (all source bytes consumed and written to dst, no dangling state).
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil voted and added 2 comments![]( "Open in Gerrit")
Votes added by Damien Neil
| Code-Review | +2 |
2 comments
return n, transform.ErrShortSrcI don't think this and the next three ErrShortSrc returns are correct. In this case, for example, we've detected a disallowed rune. Getting more data isn't going to change the rune into something allowed.
return n, transform.ErrShortSrcI think this ErrShortSrc return is correct, however, because the error here is for unterminated rules which might be terminated by following input.
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Damien Neil
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
1 comment
return n, transform.ErrShortSrcIan AlexanderI don't think this and the next three ErrShortSrc returns are correct. In this case, for example, we've detected a disallowed rune. Getting more data isn't going to change the rune into something allowed.
Done
Related details
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil added 2 comments![]( "Open in Gerrit")
return nDst, nSrc, transform.ErrShortSrcSorry, I missed this previously:
We should return ErrShortSrc here if !atEOF, because we have a partial rune which can be completed by more data.
However, we should never return ErrShortSrc when atEOF, because there's no more data coming. A reasonable thing to do would be to set r=utf8.RuneError, size=1 when when encountering a partial rune at EOF.
return n, transform.ErrShortSrcHere also: This should return ErrShortSrc when !atEOF, and errDisallowedRune otherwise.
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 2 comments![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
2 comments
Sorry, I missed this previously:
We should return ErrShortSrc here if !atEOF, because we have a partial rune which can be completed by more data.
However, we should never return ErrShortSrc when atEOF, because there's no more data coming. A reasonable thing to do would be to set r=utf8.RuneError, size=1 when when encountering a partial rune at EOF.
Done
Here also: This should return ErrShortSrc when !atEOF, and errDisallowedRune otherwise.
- Damien Neil
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Damien Neil (Gerrit)
Damien Neil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Damien Neil
| Code-Review | +2 |
1 comment
Related details
- Ian Alexander
- Nicholas Husin
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Nicholas Husin added 1 comment![]( "Open in Gerrit")
Fixes golang/go#80112Reminder to cherrypick to 1.27 release branch too.
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicholas Husin (Gerrit)
Nicholas Husin added 1 comment![]( "Open in Gerrit")
Fixes golang/go#80112Reminder to cherrypick to 1.27 release branch too.
Oops, nevermind, this is not used in std.
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mark Freeman (Gerrit)
Mark Freeman submitted the change![]( "Open in Gerrit")
Change information
secure/precis: fix short destination buffer handling in Nickname profile
The additional mapping rule for the Nickname profile (RFC 8266) requires
collapsing interior sequences of spaces. The Transform implementation
was writing an interior space to the destination buffer without first
checking if there was enough space for both the space and the next rune.
This change ensures that transform.ErrShortDst is returned if the
destination buffer is too small, preventing potential out-of-bounds
writes or incomplete copies.
Thanks to Omkhar Arasaratnam (GitHub: omkhar) for reporting this issue.
Fixes CVE-2026-56851
Fixes golang/go#80112
- M secure/precis/enforce_test.go
- M secure/precis/nickname.go
- M secure/precis/profile.go
Code-Review: +2 by Damien Neil
TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |