You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Does this issue reproduce with the latest release?
Yes, it reproduces with go version go1.15 darwin/amd64.
What did you do?
If we launch the server below (and create a /tmp/index.html with a abc\n content) it incorrectly parses Range header such that contains a single value with two minus signs and responds with an unexpected 206 reply along with an incorrect Content-Range header value.
This can be tested by invoking curl -v -H 'Range: bytes=--2' localhost:3000/.
This bug seems to be not exploitable, at least not with a sane filesystem implementation.
For more details on what causes the bug see the "Investigation" section of this issue.
package main
import"net/http"funcmain() {
http.Handle("/", http.FileServer(http.Dir("/tmp")))
http.ListenAndServe(":3000", nil)
}
What did you expect to see?
I expected to get the same reply as when an invalid range header value is sent, e.g.:
$ curl -v -H 'Range: bytes=a' localhost:3000/
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.72.0
> Accept: */*
> Range: bytes=a
>
< HTTP/1.1 416 Requested Range Not Satisfiable
< Content-Type: text/plain; charset=utf-8
< Last-Modified: Thu, 20 Aug 2020 22:43:19 GMT
< X-Content-Type-Options: nosniff
< Date: Thu, 20 Aug 2020 22:43:36 GMT
< Content-Length: 14
<
invalid range
* Connection #0 to host localhost left intact
* Closing connection 0
What did you see instead?
I got a 206 reply with an incorrect Content-Range: bytes 6-3/4 header value:
The parseRange function takes the Range header, removes the bytes= prefix from it and splits it with the "," character. Later, for each split string or rather "range" it splits it further by "-". Then, it retrieves start and end values.
When there is only one of the values, the start is an empty string. The program then goes to the first branch and parses the end value with the ParseInt(end, 10, 64) call.
To give an example, if we pass bytes=--2 Range header we end up with start="", end="-2" strings and the end is then parsed with the ParseInt(end, 10, 64) call to -2 int64 value.
The program then fills in the r httpRange fields in:
r.start=size-ir.length=size-r.start
Where, the size is just the total file size. With our example, we end up having r.start=size-(-2)=size+2 and r.length=size-(size+2)=-2. This result in what we see in the Content-Range header value.
Later, this httpRange value is used in the content.Seek call where the content's file pointer is moved past its end:
This operation does not fail (at least on my filesystem and for the bytes=--2 Range header) and a io.CopyN function is then called with a sendSize of -2 value here:
This ends up in copying no bytes from the content file pointer due to negative sendSize value and so returning no content in the request response body.
An additional observation
Additionally, the r.start = size - i calculation can overflow if we pass a very small int64 value after the bytes=-. However, this results in r.start having a negative value and making the content.Seek call fail. This can be observed in the following request/response:
$ curl -v -H 'Range: bytes=--9223372036854775808' localhost:3000/
* Trying ::1...
* TCP_NODELAY set
* Connected to localhost (::1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.72.0
> Accept: */*
> Range: bytes=--9223372036854775808
>
< HTTP/1.1 416 Requested Range Not Satisfiable
< Content-Type: text/plain; charset=utf-8
< Last-Modified: Thu, 20 Aug 2020 22:43:19 GMT
< X-Content-Type-Options: nosniff
< Date: Thu, 20 Aug 2020 23:17:59 GMT
< Content-Length: 39
<
seek /tmp/index.html: invalid argument
* Connection #0 to host localhost left intact
* Closing connection 0
How can we fix this?
It seems like the easiest fix is to validate that the resulting i value from the i, err := strconv.ParseInt(end, 10, 64) call is not negative, and if that is, return an "invalid range" error.
On the other hand, maybe the function should also allow for all possible ranges for a given architecture and so use uints64 instead? This would be nice, however it would not be as easy to keep supporting the bytes=-2
The text was updated successfully, but these errors were encountered:
disconnect3d
changed the title
net/http: FileServer incorrectly parses Range header where a single bytes value starts with two minus signs
net/http: FileServer incorrectly parses Range header when a range starts with two minus signs
Aug 20, 2020
dmitshur
added
NeedsFix
The path to resolution is known, but the work has not been done.
and removed
NeedsInvestigation
Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
labels
Sep 2, 2020
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes, it reproduces with
go version go1.15 darwin/amd64
.What did you do?
If we launch the server below (and create a
/tmp/index.html
with aabc\n
content) it incorrectly parsesRange
header such that contains a single value with two minus signs and responds with an unexpected 206 reply along with an incorrectContent-Range
header value.This can be tested by invoking
curl -v -H 'Range: bytes=--2' localhost:3000/
.This bug seems to be not exploitable, at least not with a sane filesystem implementation.
For more details on what causes the bug see the "Investigation" section of this issue.
What did you expect to see?
I expected to get the same reply as when an invalid range header value is sent, e.g.:
What did you see instead?
I got a 206 reply with an incorrect
Content-Range: bytes 6-3/4
header value:Additionally, the server removed the
Content-Length
header due to its invalid value and printed out a warning about it:Investigation
The issue comes from the following code:
go/src/net/http/fs.go
Lines 749 to 783 in e94544c
The
parseRange
function takes theRange
header, removes thebytes=
prefix from it and splits it with the","
character. Later, for each split string or rather "range" it splits it further by"-"
. Then, it retrievesstart
andend
values.When there is only one of the values, the
start
is an empty string. The program then goes to the first branch and parses theend
value with theParseInt(end, 10, 64)
call.To give an example, if we pass
bytes=--2
Range header we end up withstart="", end="-2"
strings and theend
is then parsed with theParseInt(end, 10, 64)
call to-2
int64 value.The program then fills in the
r httpRange
fields in:Where, the
size
is just the total file size. With our example, we end up havingr.start=size-(-2)=size+2
andr.length=size-(size+2)=-2
. This result in what we see in theContent-Range
header value.Later, this
httpRange
value is used in thecontent.Seek
call where thecontent
's file pointer is moved past its end:go/src/net/http/fs.go
Line 254 in e94544c
This operation does not fail (at least on my filesystem and for the
bytes=--2
Range header) and aio.CopyN
function is then called with asendSize
of-2
value here:go/src/net/http/fs.go
Line 300 in e94544c
This ends up in copying no bytes from the
content
file pointer due to negativesendSize
value and so returning no content in the request response body.An additional observation
Additionally, the
r.start = size - i
calculation can overflow if we pass a very small int64 value after thebytes=-
. However, this results inr.start
having a negative value and making thecontent.Seek
call fail. This can be observed in the following request/response:How can we fix this?
It seems like the easiest fix is to validate that the resulting
i
value from thei, err := strconv.ParseInt(end, 10, 64)
call is not negative, and if that is, return an"invalid range"
error.On the other hand, maybe the function should also allow for all possible ranges for a given architecture and so use uints64 instead? This would be nice, however it would not be as easy to keep supporting the
bytes=-2
The text was updated successfully, but these errors were encountered: