Commit d3635154 authored by Jérome Perrin's avatar Jérome Perrin

NXD xblob: don't let net/http sniff content type

This has security implications

https://gitlab.com/gitlab-org/gitlab-foss/-/issues/9079
parent 2658e1c3
......@@ -152,20 +152,43 @@ func emitBlob(w http.ResponseWriter, repopath string, refpath string) {
w.Header().Set("Content-Transfer-Encoding", "binary")
w.Header().Set("Content-Length", fmt.Sprintf("%d", size))
w.Header().Set("X-Content-Type-Options", "nosniff")
// net/http sniffs stream and automatically detects and sets
// Content-Type header. We do not have to do it ourselves.
w.Header().Set("Cache-Control", "private") // Rails sets this for IE compatibility
w.Header().Set("ETag", fmt.Sprintf(`"%s"`, sha1))
// read a little bit buffer to sniff content type, then we'll use io.CopyN to copy
// the rest
var buffsize int64 = 1024
if buffsize > size {
buffsize = size
}
buffer := make([]byte, buffsize)
_, err = io.ReadFull(queryReader, buffer)
if err != nil {
helper.Fail500(w, fmt.Errorf("io.ReadFull: %v", err))
return
}
// Serve html, javascript, etc as text/plain, using utf-8 charset because it's more likely to be correct
// than the default latin1. Keep the original content type only for images, the /raw/ of an image is
// accessed from the /blob/ view.
// See https://gitlab.com/gitlab-org/gitlab-foss/-/blob/77c7b561/app/helpers/blob_helper.rb#L138-144
contentType := http.DetectContentType(buffer)
if strings.HasPrefix(contentType, "text/") {
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
} else if strings.HasPrefix(contentType, "image/") {
w.Header().Set("Content-Type", contentType)
} else {
w.Header().Set("Content-Type", "application/octet-stream")
}
w.WriteHeader(http.StatusOK) // Don't bother with HTTP 500 from this point on, just return
// XXX better use queryStdout instead of queryReader, but we could be
// holding some tail bytes in queryReader after chat phase
_, err = io.CopyN(w, queryReader, size)
w.Write(buffer)
if size > buffsize {
_, err = io.CopyN(w, queryReader, size-buffsize)
if err != nil {
helper.LogError(fmt.Errorf("io.CopyN: %v", err))
return
}
}
// close git stdin explicitly, so it can exit cleanly
err = queryStdin.Close()
......
......@@ -814,6 +814,18 @@ func TestBlobDownload(t *testing.T) {
dl.ExpectSha1("/5f923865/files/ruby/popen.rb", "68990cc20fa74383358797a27967fa2b45d7d8f6")
dl.ExpectSha1("/874797c3/files/ruby/popen.rb", "4c266708f2bfd7ca3fed3f7ec74253f92ff3fe73")
dl.ExpectCode("/master/non-existing-file", 404)
dl.ExpectHeader("/5f923865/README", "content-type", "text/plain; charset=utf-8")
dl.ExpectHeader("/5f923865/files/html/500.html", "content-type", "text/plain; charset=utf-8")
dl.ExpectHeader("/5f923865/files/js/application.js", "content-type", "text/plain; charset=utf-8")
dl.ExpectHeader("/5f923865/files/images/logo-white.png", "content-type", "image/png")
dl.ExpectHeader("/5f923865/files/images/6049019_460s.jpg", "content-type", "image/jpeg")
dl.ExpectHeader("/5f923865/files/images/wm.svg", "content-type", "text/plain; charset=utf-8")
dl.ExpectHeader("/5f923865/Gemfile.zip", "content-type", "application/octet-stream")
dl.ExpectHeader("/5f923865/files/images/wm.svg", "content-type", "text/plain; charset=utf-8")
dl.ExpectHeader("/5f923865/VERSION", "content-type", "text/plain; charset=utf-8")
// empty file
dl.ExpectHeader("/5f923865/files/flat/path/correct/content.txt", "content-type", "text/plain; charset=utf-8")
}
func TestDeniedBlobDownload(t *testing.T) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment