Update module github.com/moby/spdystream to v0.5.1 [SECURITY] (release-0.34)#538
Conversation
Signed-off-by: redhat-renovate-bot <redhat-internal-renovate@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: redhat-renovate-bot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request updates the github.com/moby/spdystream dependency from v0.2.0 to v0.5.1 and updates the vendored code accordingly. The changes introduce frame parsing limits to mitigate potential resource exhaustion, refactor timer usage, and add thread-safety to Stream.IsFinished(). However, the vendored changes introduce critical concurrency and security issues in connection.go. Specifically, there is a data race and race condition in Connection.Ping() due to reading s.pingId outside of the lock, and a potential denial-of-service panic in Connection.handlePingFrame() because the ping channel is not deleted from the map under the lock, allowing duplicate frames to trigger concurrent channel closures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pid := s.pingId | ||
| s.pingIdLock.Lock() | ||
| s.pingLock.Lock() | ||
| if s.pingId > 0x7ffffffe { | ||
| s.pingId = s.pingId - 0x7ffffffe | ||
| } else { | ||
| s.pingId = s.pingId + 2 | ||
| } | ||
| s.pingIdLock.Unlock() | ||
| pingChan := make(chan error) | ||
| s.pingChans[pid] = pingChan | ||
| defer delete(s.pingChans, pid) | ||
| s.pingLock.Unlock() |
There was a problem hiding this comment.
Concurrency Bug & Data Race
- Data Race:
s.pingIdis read on line 283 without holdings.pingLock, but it is modified on lines 285-289 under the lock. This is a data race. - Race Condition: If two goroutines call
Ping()concurrently, both can read the sames.pingIdvalue before either acquires the lock. They will then both use the samepidto register their channels ins.pingChans, causing one to overwrite the other. This leads to lost ping responses, leaked channels, or unexpected behavior.
Acquiring the lock before reading s.pingId resolves both issues.
| pid := s.pingId | |
| s.pingIdLock.Lock() | |
| s.pingLock.Lock() | |
| if s.pingId > 0x7ffffffe { | |
| s.pingId = s.pingId - 0x7ffffffe | |
| } else { | |
| s.pingId = s.pingId + 2 | |
| } | |
| s.pingIdLock.Unlock() | |
| pingChan := make(chan error) | |
| s.pingChans[pid] = pingChan | |
| defer delete(s.pingChans, pid) | |
| s.pingLock.Unlock() | |
| s.pingLock.Lock() | |
| pid := s.pingId | |
| if s.pingId > 0x7ffffffe { | |
| s.pingId = s.pingId - 0x7ffffffe | |
| } else { | |
| s.pingId = s.pingId + 2 | |
| } | |
| pingChan := make(chan error) | |
| s.pingChans[pid] = pingChan | |
| s.pingLock.Unlock() |
| s.pingLock.Lock() | ||
| pingId := s.pingId | ||
| pingChan, pingOk := s.pingChans[frame.Id] | ||
| s.pingLock.Unlock() |
There was a problem hiding this comment.
Remote Denial of Service (DoS) Vulnerability
In handlePingFrame, the pingChan is retrieved from s.pingChans but not deleted from the map under the lock. Because ping frames are processed round-robin across multiple concurrent workers, a duplicate ping frame with the same ID can be processed concurrently. Both workers can retrieve the same pingChan and attempt to call close(pingChan) outside the lock, causing a panic (panic: close of closed channel) and crashing the process.
Deleting the entry from s.pingChans under the lock immediately when it is retrieved ensures that only one worker will find pingOk == true and close the channel.
s.pingLock.Lock()
pingId := s.pingId
pingChan, pingOk := s.pingChans[frame.Id]
if pingOk {
delete(s.pingChans, frame.Id)
}
s.pingLock.Unlock()
This PR contains the following updates:
v0.2.0→v0.5.1SpdyStream: DOS on CRI
CVE-2026-35469 / GHSA-pc3f-x583-g7j2 / GO-2026-4958
More information
Details
The SPDY/3 frame parser in spdystream does not validate
attacker-controlled counts and lengths before allocating memory. A
remote peer that can send SPDY frames to a service using spdystream can
cause the process to allocate gigabytes of memory with a small number of
malformed control frames, leading to an out-of-memory crash.
Three allocation paths in the receive side are affected:
numSettingsfrom the payload and allocates a slice of that sizewithout checking it against the declared frame length. An attacker
can set
numSettingsto a value far exceeding the actual payload,triggering a large allocation before any setting data is read.
parseHeaderValueBlockreads a 32-bitnumHeadersfrom the decompressed header block and allocates anhttp.Headermap of that size with no upper bound.read as 32-bit integers and used directly as allocation sizes with
no validation.
Because SPDY header blocks are zlib-compressed, a small on-the-wire
payload can decompress into attacker-controlled bytes that the parser
interprets as 32-bit counts and lengths. A single crafted frame is
enough to exhaust process memory.
Impact
Any program that accepts SPDY connections using spdystream -- directly
or through a dependent library -- is affected. A remote peer that can
send SPDY frames to the service can crash the process with a single
crafted SPDY control frame, causing denial of service.
Affected versions
github.com/moby/spdystream<= v0.5.0Fix
v0.5.1 addresses the receive-side allocation bugs and adds related
hardening:
Core fixes:
checks that
numSettingsis consistent with the declared framelength (
numSettings <= (length-4)/8) before allocating.parseHeaderValueBlockenforces a maximumnumber of headers per frame (default: 1000).
lengths are checked against a per-field size limit (default: 1 MiB)
before allocation.
now closes the underlying
net.Connwhen it encounters anInvalidControlFrameerror, preventing further exploitation on thesame connection.
Additional hardening:
that payloads fit within the 24-bit length field, preventing the
library from producing invalid frames.
Configurable limits:
NewConnectionWithOptionsorthe lower-level
spdy.NewFramerWithOptionswith functional options:WithMaxControlFramePayloadSize,WithMaxHeaderFieldSize, andWithMaxHeaderCount.Severity
CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:NReferences
This data is provided by OSV and the GitHub Advisory Database (CC-BY 4.0).
Uncontrolled resource consumption when parsing SPDY frames in github.com/moby/spdystream
CVE-2026-35469 / GHSA-pc3f-x583-g7j2 / GO-2026-4958
More information
Details
The SPDY/3 frame parser in spdystream does not validate attacker-controlled counts and lengths before allocating memory. A remote peer that can send SPDY frames to a service using spdystream can cause the process to allocate gigabytes of memory with a small number of malformed control frames, leading to an out-of-memory crash.
Three allocation paths in the receive side are affected:
Because SPDY header blocks are zlib-compressed, a small on-the-wire payload can decompress into attacker-controlled bytes that the parser interprets as 32-bit counts and lengths. A single crafted frame is enough to exhaust process memory.
Severity
Unknown
References
This data is provided by OSV and the Go Vulnerability Database (CC-BY 4.0).
Release Notes
moby/spdystream (github.com/moby/spdystream)
v0.5.1Compare Source
What's Changed
Security
Fix memory amplification in SPDY frame parsing leads to denial of service (CVE-2026-35469 / GHSA-pc3f-x583-g7j2)
Changes
Full Changelog: moby/spdystream@v0.5.0...v0.5.1
v0.5.0: [v0.5.0] Avoid leaking timeout timer channels and update github actionsCompare Source
What's Changed
Full Changelog: moby/spdystream@v0.4.0...v0.5.0
v0.4.0: [v0.4.0] fix goroutine leak and remove unused codeCompare Source
What's Changed
New Contributors
Full Changelog: moby/spdystream@v0.3.0...v0.4.0
v0.3.0: [v0.3.0] Release with fixes for a race conditionCompare Source
What's Changed
New Contributors
Full Changelog: moby/spdystream@v0.2.0...v0.3.0
Configuration
📅 Schedule: (UTC)
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR has been generated by Mend Renovate.