diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 7d061495e..833721fe8 100644 --- a/cmd/api-errors.go +++ b/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: { diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index facef3fc7..ee2f11672 100644 --- a/cmd/generic-handlers.go +++ b/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) +} diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index ce78e9265..b313b5e6c 100644 --- a/cmd/object-api-utils.go +++ b/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 } diff --git a/cmd/object-api-utils_test.go b/cmd/object-api-utils_test.go index abf3560da..8da3c09fe 100644 --- a/cmd/object-api-utils_test.go +++ b/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}, } diff --git a/cmd/object-handlers_test.go b/cmd/object-handlers_test.go index f6b10539d..ddc6d9091 100644 --- a/cmd/object-handlers_test.go +++ b/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. diff --git a/cmd/routers.go b/cmd/routers.go index 734f643f3..af0cd1654 100644 --- a/cmd/routers.go +++ b/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 diff --git a/cmd/server_test.go b/cmd/server_test.go index 53e075858..157990f3d 100644 --- a/cmd/server_test.go +++ b/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 diff --git a/cmd/storage-errors.go b/cmd/storage-errors.go index accb27240..ee1729dce 100644 --- a/cmd/storage-errors.go +++ b/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") diff --git a/cmd/test-utils_test.go b/cmd/test-utils_test.go index 588b26a27..be3a4caeb 100644 --- a/cmd/test-utils_test.go +++ b/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)