hyperium / headers

Typed HTTP Headers from hyper
https://hyper.rs
MIT License
164 stars 83 forks source link

OriginOrNull::try_from is too lenient #57

Open pshaughn opened 4 years ago

pshaughn commented 4 years ago

https://fetch.spec.whatwg.org/#origin-header has a very strict ABNF. Strings ending in # or / shouldn't be allowed. Because OriginOrNull processes strings via url parsing (https://github.com/hyperium/headers/blob/c6be8bab9c9852581bccd03f89cdbe431195ca1c/src/common/origin.rs#L134), these elements are normalized away, leading it to produce an allegedly valid header out of an invalid string.

According to https://github.com/web-platform-tests/wpt/blob/master/cors/remote-origin.htm uppercasing the scheme as HTTP: should likewise be considered invalid, although that's not as obvious from the grammar.

seanmonstar commented 4 years ago

Are there some header values that we can use for tests of this? On a cursory look of the code, it should reject URLs that have more than scheme + authority.

pshaughn commented 4 years ago

The failing ones are: https://github.com/web-platform-tests/wpt/blob/c7df60fa0b11d94487f56d8bde44d979a6a8370e/cors/remote-origin.htm#L85 shouldFail("<origin>" + "#") https://github.com/web-platform-tests/wpt/blob/c7df60fa0b11d94487f56d8bde44d979a6a8370e/cors/remote-origin.htm#L83 shouldFail("<origin>" + "/") https://github.com/web-platform-tests/wpt/blob/c7df60fa0b11d94487f56d8bde44d979a6a8370e/cors/remote-origin.htm#L91 shouldFail("<PROTOCOL>//<host>") where PROTOCOL means the protocol has been uppercased.

ghostd commented 4 years ago

The same seems apply for the Access-Control-Allow-Origin header. The tests in WTP: https://github.com/web-platform-tests/wpt/blob/b1997c4b395772d5eb212656e995926f99ec8899/cors/origin.htm#L64 https://github.com/web-platform-tests/wpt/blob/b1997c4b395772d5eb212656e995926f99ec8899/cors/origin.htm#L66 https://github.com/web-platform-tests/wpt/blob/b1997c4b395772d5eb212656e995926f99ec8899/cors/origin.htm#L72