Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

MemoryPoolIterator.PeekLong() sometimes returns invalid values #1183

Closed
halter73 opened this issue Oct 27, 2016 · 0 comments
Closed

MemoryPoolIterator.PeekLong() sometimes returns invalid values #1183

halter73 opened this issue Oct 27, 2016 · 0 comments
Assignees
Milestone

Comments

@halter73
Copy link
Member

MemoryPool.PeekLong() is used by GetKnownMethod and GetKnownVersion methods which determine the HTTP verb and HTTP version from the start line respectively.

The primary issue occurs when the iterator is at the very end of a block causing PeekLong() to attempt a 64 bit right shift. Instead of evaluating to 0 as the code in PeekLong() expects, the 64 bit right shift evaluates to blockLong since the least significant 6 bits of 64 are all 0. The blockLong value which should be completely ignored since it only includes bytes before the iterator index is then bitwise-ored into the return value.

Another issue is that the right shift on blockLong in PeekLong() is an arithmetic shift since it is done on a signed type which will fill in high-order empty bits to the sign bit instead of 0 as expected.

Lastly PeekLong() returns -1 as a sentinel value indicating a valid long can't be returned because there aren't enough bytes following the iterator even though theoretically -1 is a valid value. Fortunately this doesn't appear to ever affect the functional behavior of Kestrel since -1 would always be unexpected and treated the same way as there not being enough data.

GetKnownMethod will allocate a new (correct) Method string if PeekLong() doesn't return a long matching a known HTTP verb, but there's a small chance that an incorrect return value could match another known HTTP verb causing the wrong Method string to be returned from IHttpRequestFeature.Method.

GetKnownVersion behaves similarly to GetKnownMethod and also has a small chance of parsing the wrong HTTP version (e.g. "HTTP/1.1" instead of "HTTP/1.0"). Sarting in 1.1.0-preview1 if the HTTP version starts immediate following a block boundary, Kestrel will fail with a 505 because it no longer does the fallback string comparison

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant