Merge pull request #1186 from cyli/limit-readall-size

Do not ioutil.ReadAll anything without limiting the size.

storage/httpstore.go

@@ -28,6 +28,13 @@ import (
 	"github.com/docker/notary/tuf/validation"
 )
 
+const (
+	// MaxErrorResponseSize is the maximum size for an error message - 1KiB
+	MaxErrorResponseSize int64 = 1 << 10
+	// MaxKeySize is the maximum size for a stored TUF key - 256KiB
+	MaxKeySize = 256 << 10
+)
+
 // ErrServerUnavailable indicates an error from the server. code allows us to
 // populate the http error we received
 type ErrServerUnavailable struct {
@@ -128,7 +135,8 @@ func NewHTTPStore(baseURL, metaPrefix, metaExtension, keyExtension string, round
 }
 
 func tryUnmarshalError(resp *http.Response, defaultError error) error {
-	bodyBytes, err := ioutil.ReadAll(resp.Body)
+	b := io.LimitReader(resp.Body, MaxErrorResponseSize)
+	bodyBytes, err := ioutil.ReadAll(b)
 	if err != nil {
 		return defaultError
 	}
@@ -319,7 +327,8 @@ func (s HTTPStore) GetKey(role data.RoleName) ([]byte, error) {
 	if err := translateStatusToError(resp, role.String()+" key"); err != nil {
 		return nil, err
 	}
-	body, err := ioutil.ReadAll(resp.Body)
+	b := io.LimitReader(resp.Body, MaxKeySize)
+	body, err := ioutil.ReadAll(b)
 	if err != nil {
 		return nil, err
 	}
@@ -344,7 +353,8 @@ func (s HTTPStore) RotateKey(role data.RoleName) ([]byte, error) {
 	if err := translateStatusToError(resp, role.String()+" key"); err != nil {
 		return nil, err
 	}
-	body, err := ioutil.ReadAll(resp.Body)
+	b := io.LimitReader(resp.Body, MaxKeySize)
+	body, err := ioutil.ReadAll(b)
 	if err != nil {
 		return nil, err
 	}

storage/httpstore_test.go

@@ -240,6 +240,31 @@ func TestTranslateErrorsWhenCannotParse400(t *testing.T) {
 	}
 }
 
+// Cut off error reading after a certain size
+func TestTranslateErrorsLimitsErrorSize(t *testing.T) {
+	// if the error message itself is the max error size, then extra JSON surrounding it will put it over
+	// the top
+	msg := make([]byte, MaxErrorResponseSize)
+	for i := range msg {
+		msg[i] = 'a'
+	}
+
+	serialObj, err := validation.NewSerializableError(validation.ErrBadRoot{Msg: string(msg)})
+	require.NoError(t, err)
+	serialization, err := json.Marshal(serialObj)
+	require.NoError(t, err)
+	errorBody := bytes.NewBuffer([]byte(fmt.Sprintf(
+		`{"errors": [{"otherstuff": "what", "detail": %s}]}`,
+		string(serialization))))
+	errorResp := http.Response{
+		StatusCode: http.StatusBadRequest,
+		Body:       ioutil.NopCloser(errorBody),
+	}
+
+	err = translateStatusToError(&errorResp, "")
+	require.IsType(t, ErrInvalidOperation{}, err)
+}
+
 func TestHTTPStoreRemoveAll(t *testing.T) {
 	// Set up a simple handler and server for our store, just check that a non-error response back is fine
 	handler := func(w http.ResponseWriter, r *http.Request) {}
@@ -323,6 +348,27 @@ func TestHTTPStoreGetKey(t *testing.T) {
 	require.Equal(t, "FAIL", err.Error())
 }
 
+func TestHTTPStoreGetRotateKeySizeLimited(t *testing.T) {
+	tooLarge := make([]byte, MaxKeySize+10)
+	for i := range tooLarge {
+		tooLarge[i] = 'a'
+	}
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, "/metadata/snapshot.key", r.URL.Path)
+		w.Write(tooLarge)
+	}
+	server := httptest.NewServer(http.HandlerFunc(handler))
+	defer server.Close()
+	store, err := NewHTTPStore(server.URL, "metadata", "json", "key", http.DefaultTransport)
+	require.NoError(t, err)
+
+	for _, downloadFunc := range []func(data.RoleName) ([]byte, error){store.RotateKey, store.GetKey} {
+		gotten, err := downloadFunc(data.CanonicalSnapshotRole)
+		require.NoError(t, err)
+		require.Equal(t, tooLarge[:MaxKeySize], gotten)
+	}
+}
+
 func TestHTTPOffline(t *testing.T) {
 	s, err := NewHTTPStore("https://localhost/", "", "", "", nil)
 	require.NoError(t, err)