[
https://issues.apache.org/jira/browse/THRIFT-6262?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer updated THRIFT-6262:
-------------------------------
Description:
{{lib/go/thrift/header_transport.go}}, in {{Flush()}} ({{:713}}):
{code:go}
binary.BigEndian.PutUint32(buf, uint32(payload.Len()))
if _, err := t.transport.Write(buf); err != nil {
return NewTTransportExceptionFromError(err)
}
{code}
There is no upper bound on the write side at all -- the configured
{{MaxFrameSize}} is applied when reading, not when flushing -- so this
transport will happily emit a frame no conforming peer will accept.
On a 64-bit build the missing bound also turns into silent corruption:
{{payload.Len()}} is an {{int}}, which is 64-bit there, and the conversion to
{{uint32}} wraps, so a payload of 4 GiB + n writes a length of n and the peer
reads a frame that does not match what follows. (On a 32-bit build {{int}} is
32-bit and {{payload.Len()}} cannot exceed {{MaxInt32}}, so only the missing
bound applies.)
The read side refuses what it cannot accept; the write side should not produce
what no peer can read.
h2. Suggested fix
Check {{payload.Len()}} against {{t.cfg.GetMaxFrameSize()}} (and against
{{math.MaxUint32}}) before the conversion and return a {{TTransportException}}
of type {{SIZE_LIMIT}} rather than writing a truncated length.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
was:
{{lib/go/thrift/header_transport.go}}, in {{Flush()}} ({{:713}}):
{code:go}
binary.BigEndian.PutUint32(buf, uint32(payload.Len()))
if _, err := t.transport.Write(buf); err != nil {
return NewTTransportExceptionFromError(err)
}
{code}
{{payload.Len()}} is an {{int}}, 64-bit on every platform Thrift supports in
practice. The conversion to {{uint32}} wraps silently, so a payload of 4 GiB +
n writes a length of n and the peer reads a frame that does not match what
follows. There is no upper bound on the write side at all: the configured
{{MaxFrameSize}} is applied when reading, not when flushing.
The read side refuses what it cannot accept; the write side should not produce
what no peer can read.
h2. Suggested fix
Check {{payload.Len()}} against {{t.cfg.GetMaxFrameSize()}} (and against
{{math.MaxUint32}}) before the conversion and return a {{TTransportException}}
of type {{SIZE_LIMIT}} rather than writing a truncated length.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
> Go: THeaderTransport.Flush truncates the frame length instead of refusing an
> oversized frame
> --------------------------------------------------------------------------------------------
>
> Key: THRIFT-6262
> URL: https://issues.apache.org/jira/browse/THRIFT-6262
> Project: Thrift
> Issue Type: Bug
> Components: Go - Library
> Reporter: Jens Geyer
> Priority: Major
>
> {{lib/go/thrift/header_transport.go}}, in {{Flush()}} ({{:713}}):
> {code:go}
> binary.BigEndian.PutUint32(buf, uint32(payload.Len()))
> if _, err := t.transport.Write(buf); err != nil {
> return NewTTransportExceptionFromError(err)
> }
> {code}
> There is no upper bound on the write side at all -- the configured
> {{MaxFrameSize}} is applied when reading, not when flushing -- so this
> transport will happily emit a frame no conforming peer will accept.
> On a 64-bit build the missing bound also turns into silent corruption:
> {{payload.Len()}} is an {{int}}, which is 64-bit there, and the conversion to
> {{uint32}} wraps, so a payload of 4 GiB + n writes a length of n and the peer
> reads a frame that does not match what follows. (On a 32-bit build {{int}} is
> 32-bit and {{payload.Len()}} cannot exceed {{MaxInt32}}, so only the missing
> bound applies.)
> The read side refuses what it cannot accept; the write side should not
> produce what no peer can read.
> h2. Suggested fix
> Check {{payload.Len()}} against {{t.cfg.GetMaxFrameSize()}} (and against
> {{math.MaxUint32}}) before the conversion and return a
> {{TTransportException}} of type {{SIZE_LIMIT}} rather than writing a
> truncated length.
> _Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens
> Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
