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

x/net/http2: sendWindowUpdate may send invalid window size increment #31170

Open
anmonteiro opened this issue Mar 31, 2019 · 1 comment

Comments

@anmonteiro
Copy link

commented Mar 31, 2019

Disclaimer: I didn't run into an issue here, but I'm working on an HTTP/2 implementation and saw something that could possibly not be according to the spec.

The code linked below contains a loop that sends multiple WINDOW_UPDATE frames if the window size increment is higher than the allowed by the HTTP/2 spec. It then sends a last WINDOW_UPDATE frame after the loop has exited.

https://github.com/golang/net/blob/74de082e2cca95839e88aa0aeee5aadf6ce7710f/http2/server.go#L2189-L2193

However, because the check reads: for n >= maxUint31, the last frame sent could contain a window size increment of 0 if the initial n is a multiple of 2^31 - 1. That is not allowed by the HTTP/2 spec:

From: https://httpwg.org/specs/rfc7540.html#rfc.section.6.9

A receiver MUST treat the receipt of a WINDOW_UPDATE frame with an flow-control window increment of 0 as a stream error (Section 5.4.2) of type PROTOCOL_ERROR; errors on the connection flow-control window MUST be treated as a connection error (Section 5.4.1).

I think the fix here would be just to change the >= check to >. I suspect that the reason nobody ran into this before is that it's already extremely unlikely that all the conditions for this bug to manifest are actually met in the wild.

P.S.: The questions in the issue template didn't really apply to this issue. Apologies in advance if this is not very helpful.

@gopherbot gopherbot added this to the Unreleased milestone Mar 31, 2019

@anmonteiro anmonteiro changed the title x/net: HTTP/2 `sendWindowUpdate` may send invalid window size increment x/net/http2: `sendWindowUpdate` may send invalid window size increment Mar 31, 2019

@andybons

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

@andybons andybons changed the title x/net/http2: `sendWindowUpdate` may send invalid window size increment x/net/http2: sendWindowUpdate may send invalid window size increment Apr 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.