server: Validate path for bad components in a handler. (#4170)

cmd/api-errors.go

@@ -139,6 +139,7 @@ const (
 	ErrObjectExistsAsDirectory
 	ErrPolicyNesting
 	ErrInvalidObjectName
+	ErrInvalidResourceName
 	ErrServerNotInitialized
 	// Add new extended error codes here.
 	// Please open a https://github.com/minio/minio/issues before adding
@@ -588,7 +589,12 @@ var errorCodeResponse = map[APIErrorCode]APIError{
 	},
 	ErrInvalidObjectName: {
 		Code:           "XMinioInvalidObjectName",
-		Description:    "Object name contains unsupported characters. Unsupported characters are `^*|\\\"",
+		Description:    "Object name contains unsupported characters.",
+		HTTPStatusCode: http.StatusBadRequest,
+	},
+	ErrInvalidResourceName: {
+		Code:           "XMinioInvalidResourceName",
+		Description:    "Resource name contains bad components such as \"..\" or \".\".",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
 	ErrServerNotInitialized: {

cmd/generic-handlers.go

@@ -419,3 +419,52 @@ func (h httpStatsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// Update http statistics
 	globalHTTPStats.updateStats(r, ww, durationSecs)
 }
+
+// pathValidityHandler validates all the incoming paths for
+// any bad components and rejects them.
+type pathValidityHandler struct {
+	handler http.Handler
+}
+
+func setPathValidityHandler(h http.Handler) http.Handler {
+	return pathValidityHandler{handler: h}
+}
+
+// Bad path components to be rejected by the path validity handler.
+const (
+	dotdotComponent = ".."
+	dotComponent    = "."
+)
+
+// Check if the incoming path has bad path components,
+// such as ".." and "."
+func hasBadPathComponent(path string) bool {
+	path = strings.TrimSpace(path)
+	for _, p := range strings.Split(path, slashSeparator) {
+		switch strings.TrimSpace(p) {
+		case dotdotComponent:
+			return true
+		case dotComponent:
+			return true
+		}
+	}
+	return false
+}
+
+func (h pathValidityHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	// Check for bad components in URL path.
+	if hasBadPathComponent(r.URL.Path) {
+		writeErrorResponse(w, ErrInvalidResourceName, r.URL)
+		return
+	}
+	// Check for bad components in URL query values.
+	for _, vv := range r.URL.Query() {
+		for _, v := range vv {
+			if hasBadPathComponent(v) {
+				writeErrorResponse(w, ErrInvalidResourceName, r.URL)
+				return
+			}
+		}
+	}
+	h.handler.ServeHTTP(w, r)
+}

cmd/object-api-utils.go

@@ -131,6 +131,9 @@ func IsValidObjectName(object string) bool {
 // IsValidObjectPrefix verifies whether the prefix is a valid object name.
 // Its valid to have a empty prefix.
 func IsValidObjectPrefix(object string) bool {
+	if hasBadPathComponent(object) {
+		return false
+	}
 	if len(object) > 1024 {
 		return false
 	}

cmd/object-api-utils_test.go

@@ -100,12 +100,22 @@ func TestIsValidObjectName(t *testing.T) {
 		{"contains-|-pipe", true},
 		{"contains-\"-quote", true},
 		{"contains-`-tick", true},
+		{"..test", true},
+		{".. test", true},
+		{". test", true},
+		{".test", true},
 		{"There are far too many object names, and far too few bucket names!", true},
 		// cases for which test should fail.
 		// passing invalid object names.
 		{"", false},
 		{"a/b/c/", false},
 		{"/a/b/c", false},
+		{"../../etc", false},
+		{"../../", false},
+		{"/../../etc", false},
+		{" ../etc", false},
+		{"./././", false},
+		{"./etc", false},
 		{"contains-\\-backslash", false},
 		{string([]byte{0xff, 0xfe, 0xfd}), false},
 	}

cmd/object-handlers_test.go

@@ -316,6 +316,58 @@ func testAPIGetObjectHandler(obj ObjectLayer, instanceType, bucketName string, a
 			expectedContent:    encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidAccessKeyID), getGetObjectURL("", bucketName, objectName))),
 			expectedRespStatus: http.StatusForbidden,
 		},
+		// Test case - 7.
+		// Case with bad components in object name.
+		{
+			bucketName: bucketName,
+			objectName: "../../etc",
+			byteRange:  "",
+			accessKey:  credentials.AccessKey,
+			secretKey:  credentials.SecretKey,
+
+			expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidObjectName),
+				getGetObjectURL("", bucketName, "../../etc"))),
+			expectedRespStatus: http.StatusBadRequest,
+		},
+		// Test case - 8.
+		// Case with strange components but returning error as not found.
+		{
+			bucketName: bucketName,
+			objectName: ". ./. ./etc",
+			byteRange:  "",
+			accessKey:  credentials.AccessKey,
+			secretKey:  credentials.SecretKey,
+
+			expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrNoSuchKey),
+				"/"+bucketName+"/"+". ./. ./etc")),
+			expectedRespStatus: http.StatusNotFound,
+		},
+		// Test case - 9.
+		// Case with bad components in object name.
+		{
+			bucketName: bucketName,
+			objectName: ". ./../etc",
+			byteRange:  "",
+			accessKey:  credentials.AccessKey,
+			secretKey:  credentials.SecretKey,
+
+			expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidObjectName),
+				"/"+bucketName+"/"+". ./../etc")),
+			expectedRespStatus: http.StatusBadRequest,
+		},
+		// Test case - 10.
+		// Case with proper components
+		{
+			bucketName: bucketName,
+			objectName: "etc/path/proper/.../etc",
+			byteRange:  "",
+			accessKey:  credentials.AccessKey,
+			secretKey:  credentials.SecretKey,
+
+			expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrNoSuchKey),
+				getGetObjectURL("", bucketName, "etc/path/proper/.../etc"))),
+			expectedRespStatus: http.StatusNotFound,
+		},
 	}
 
 	// Iterating over the cases, fetching the object validating the response.
@@ -346,7 +398,7 @@ func testAPIGetObjectHandler(obj ObjectLayer, instanceType, bucketName string, a
 		}
 		// Verify whether the bucket obtained object is same as the one created.
 		if !bytes.Equal(testCase.expectedContent, actualContent) {
-			t.Errorf("Test %d: %s: Object content differs from expected value.: %s", i+1, instanceType, string(actualContent))
+			t.Errorf("Test %d: %s: Object content differs from expected value %s, got %s", i+1, instanceType, testCase.expectedContent, string(actualContent))
 		}
 
 		// Verify response of the V2 signed HTTP request.

cmd/routers.go

@@ -85,6 +85,8 @@ func configureServerHandler(endpoints EndpointList) (http.Handler, error) {
 
 	// List of some generic handlers which are applied for all incoming requests.
 	var handlerFns = []HandlerFunc{
+		// Validate all the incoming paths.
+		setPathValidityHandler,
 		// Network statistics
 		setHTTPStatsHandler,
 		// Limits all requests size to a maximum fixed limit

cmd/server_test.go

@@ -170,7 +170,7 @@ func (s *TestSuiteCommon) TestObjectDir(c *C) {
 	response, err = client.Do(request)
 
 	c.Assert(err, IsNil)
-	verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters. Unsupported characters are `^*|\\\"", http.StatusBadRequest)
+	verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters.", http.StatusBadRequest)
 }
 
 func (s *TestSuiteCommon) TestBucketSQSNotificationAMQP(c *C) {
@@ -1245,7 +1245,7 @@ func (s *TestSuiteCommon) TestPutObjectLongName(c *C) {
 
 	response, err = client.Do(request)
 	c.Assert(err, IsNil)
-	verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters. Unsupported characters are `^*|\\\"", http.StatusBadRequest)
+	verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters.", http.StatusBadRequest)
 }
 
 // TestNotBeAbleToCreateObjectInNonexistentBucket - Validates the error response

cmd/storage-errors.go

@@ -48,6 +48,9 @@ var errFileNotFound = errors.New("file not found")
 // errFileNameTooLong - given file name is too long than supported length.
 var errFileNameTooLong = errors.New("file name too long")
 
+// errFileComponentInvalid - given file name has invalid components.
+var errFileComponentInvalid = errors.New("file name has invalid components")
+
 // errVolumeExists - cannot create same volume again.
 var errVolumeExists = errors.New("volume already exists")
 

cmd/test-utils_test.go

@@ -318,7 +318,7 @@ func StartTestServer(t TestErrHandler, instanceType string) TestServer {
 // The object Layer will be a temp back used for testing purpose.
 func initTestStorageRPCEndPoint(endpoints EndpointList) http.Handler {
 	// Initialize router.
-	muxRouter := router.NewRouter()
+	muxRouter := router.NewRouter().SkipClean(true)
 	registerStorageRPCRouters(muxRouter, endpoints)
 	return muxRouter
 }
@@ -389,7 +389,7 @@ func StartTestPeersRPCServer(t TestErrHandler, instanceType string) TestServer {
 	testRPCServer.Obj = objLayer
 	globalObjLayerMutex.Unlock()
 
-	mux := router.NewRouter()
+	mux := router.NewRouter().SkipClean(true)
 	// need storage layer for bucket config storage.
 	registerStorageRPCRouters(mux, endpoints)
 	// need API layer to send requests, etc.
@@ -2154,7 +2154,7 @@ func registerAPIFunctions(muxRouter *router.Router, objLayer ObjectLayer, apiFun
 func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Handler {
 	// initialize a new mux router.
 	// goriilla/mux is the library used to register all the routes and handle them.
-	muxRouter := router.NewRouter()
+	muxRouter := router.NewRouter().SkipClean(true)
 	if len(apiFunctions) > 0 {
 		// Iterate the list of API functions requested for and register them in mux HTTP handler.
 		registerAPIFunctions(muxRouter, objLayer, apiFunctions...)
@@ -2171,7 +2171,7 @@ func initTestWebRPCEndPoint(objLayer ObjectLayer) http.Handler {
 	globalObjLayerMutex.Unlock()
 
 	// Initialize router.
-	muxRouter := router.NewRouter()
+	muxRouter := router.NewRouter().SkipClean(true)
 	registerWebRouter(muxRouter)
 	return muxRouter
 }
@@ -2179,7 +2179,7 @@ func initTestWebRPCEndPoint(objLayer ObjectLayer) http.Handler {
 // Initialize browser RPC endpoint.
 func initTestBrowserPeerRPCEndPoint() http.Handler {
 	// Initialize router.
-	muxRouter := router.NewRouter()
+	muxRouter := router.NewRouter().SkipClean(true)
 	registerBrowserPeerRPCRouter(muxRouter)
 	return muxRouter
 }
@@ -2233,7 +2233,7 @@ func StartTestS3PeerRPCServer(t TestErrHandler) (TestServer, []string) {
 	globalObjLayerMutex.Unlock()
 
 	// Register router on a new mux
-	muxRouter := router.NewRouter()
+	muxRouter := router.NewRouter().SkipClean(true)
 	err = registerS3PeerRPCRouter(muxRouter)
 	if err != nil {
 		t.Fatalf("%s", err)