Skip to content

Commit

Permalink
http2: remove suspicious uint32->v conversion in frame code
Browse files Browse the repository at this point in the history
Function maxHeaderStringLen(...) uses uint32(int(v)) == v check to validate
if length will fit in the int type.
This check is a no-op on any architecture because int type always has at
least 32 bits, so we can potentially encounter negative return values from
maxHeaderStringLen(...) function. This can be bad as this outcome clearly
breaks code intention and maybe some further code invariants.

This patch replaces uint32(int(v)) == v check with more robust and simpler
int(v) > 0 validation which is correct for our case when we operating with
uint32

Fixes golang/go#64961

Change-Id: I31f95709df9d25593ade3200696ac5cef9f88652
Reviewed-on: https://go-review.googlesource.com/c/net/+/554235
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
  • Loading branch information
sivukhin authored and gopherbot committed Jan 10, 2024
1 parent 26b646e commit 07e05fd
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions http2/frame.go
Original file line number Diff line number Diff line change
Expand Up @@ -1510,13 +1510,12 @@ func (mh *MetaHeadersFrame) checkPseudos() error {
}

func (fr *Framer) maxHeaderStringLen() int {
v := fr.maxHeaderListSize()
if uint32(int(v)) == v {
return int(v)
v := int(fr.maxHeaderListSize())
if v < 0 {
// If maxHeaderListSize overflows an int, use no limit (0).
return 0
}
// They had a crazy big number for MaxHeaderBytes anyway,
// so give them unlimited header lengths:
return 0
return v
}

// readMetaFrame returns 0 or more CONTINUATION frames from fr and
Expand Down

0 comments on commit 07e05fd

Please sign in to comment.