Merge pull request from GHSA-jg2r-qf99-4wvr

add content type avoidance check

CHANGELOG.adoc

@@ -19,6 +19,9 @@ toc::[]
 
 == HEAD
 
+== v2.1.1 - 2019-11-11
+
+*   Security fixes / content-type validation
 *   Add `ProxyFromEnvironment` support. This uses HTTP proxies directed by the
     `HTTP_PROXY` and `NO_PROXY` (or `http_proxy` and `no_proxy`) environment
     variables. See {link-proxy-from-env} for more info.

pkg/camo/helpers_test.go

@@ -88,7 +88,7 @@ func bodyAssert(t *testing.T, expected string, resp *http.Response) {
 func headerAssert(t *testing.T, expected, name string, resp *http.Response) {
 	assert.Equal(
 		t, expected, resp.Header.Get(name),
-		"Expected response header 'Server' not found",
+		"Expected response header mismatch",
 	)
 }
 

pkg/camo/proxy.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"mime"
 	"net"
 	"net/http"
 	"net/url"
@@ -218,10 +219,12 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
+	var responseContentType string
 	switch resp.StatusCode {
 	case 200, 206:
 		contentType := resp.Header.Get("Content-Type")
 
+		// early abort if content type is empty. avoids empty mime parsing overhead.
 		if contentType == "" {
 			if mlog.HasDebug() {
 				mlog.Debug("Empty content-type returned")
@@ -230,13 +233,36 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 			return
 		}
 
-		if !p.acceptTypesFilter.CheckPath(contentType) {
+		// content-type: image/png; charset=...
+		// be careful of malformed content type records. apparently some browsers
+		// only loosely validate this, and tend to pick whichever one "seems correct",
+		// or have a "default fallback" such as text/html, which would be insecure in
+		// this context.
+		// content-type: image/png, text/html; charset=...
+		mediatype, param, err := mime.ParseMediaType(contentType)
+		if err != nil || !p.acceptTypesFilter.CheckPath(mediatype) {
 			if mlog.HasDebug() {
 				mlog.Debugm("Unsupported content-type returned", mlog.Map{"type": u})
 			}
 			http.Error(w, "Unsupported content-type returned", http.StatusBadRequest)
 			return
 		}
+
+		// add params back in, as certain content types have various optional and/or
+		// required parameters.
+		// refs: https://www.iana.org/assignments/media-types/media-types.xhtml
+		responseContentType = mime.FormatMediaType(mediatype, param)
+
+		// also check if the parsed content type is empty, just to be safe.
+		// note: round trip of mediatype and params _should_ be fine, but guard
+		// against implementation changes or bugs.
+		if responseContentType == "" {
+			if mlog.HasDebug() {
+				mlog.Debug("Unsupported content-type returned")
+			}
+			http.Error(w, "Unsupported content-type returned", http.StatusBadRequest)
+			return
+		}
 	case 300:
 		http.Error(w, "Multiple choices not supported", http.StatusNotFound)
 		return
@@ -264,6 +290,8 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 	h := w.Header()
 	p.copyHeaders(&h, &resp.Header, &ValidRespHeaders)
+	// set content type based on parsed content type, not originally supplied
+	h.Set("content-type", responseContentType)
 	w.WriteHeader(resp.StatusCode)
 
 	// get a []byte from bufpool, and put it back on defer

pkg/camo/proxy_test.go

@@ -101,6 +101,40 @@ func TestBadContentType(t *testing.T) {
 	assert.Nil(t, err)
 }
 
+func TestContentTypeParams(t *testing.T) {
+	t.Parallel()
+	testURL := "http://httpbin.org/response-headers?Content-Type=image/svg%2Bxml;charset%3DUTF-8"
+	resp, err := makeTestReq(testURL, 200, camoConfig)
+
+	assert.Nil(t, err)
+	if assert.Nil(t, err) {
+		headerAssert(t, "image/svg+xml; charset=UTF-8", "content-type", resp)
+	}
+}
+
+func TestBadContentTypeSmuggle(t *testing.T) {
+	t.Parallel()
+	testURL := "http://httpbin.org/response-headers?Content-Type=image/png,%20text/html;%20charset%3DUTF-8"
+	_, err := makeTestReq(testURL, 400, camoConfig)
+	assert.Nil(t, err)
+
+	testURL = "http://httpbin.org/response-headers?Content-Type=image/png,text/html;%20charset%3DUTF-8"
+	_, err = makeTestReq(testURL, 400, camoConfig)
+	assert.Nil(t, err)
+
+	testURL = "http://httpbin.org/response-headers?Content-Type=image/png%20text/html"
+	_, err = makeTestReq(testURL, 400, camoConfig)
+	assert.Nil(t, err)
+
+	testURL = "http://httpbin.org/response-headers?Content-Type=image/png%;text/html"
+	_, err = makeTestReq(testURL, 400, camoConfig)
+	assert.Nil(t, err)
+
+	testURL = "http://httpbin.org/response-headers?Content-Type=image/png;%20charset%3DUTF-8;text/html"
+	_, err = makeTestReq(testURL, 400, camoConfig)
+	assert.Nil(t, err)
+}
+
 func TestXForwardedFor(t *testing.T) {
 	t.Parallel()