Skip to content

Commit f420218

Browse files
harshavardhanaharshavardhana
committed
server: Validate path for bad components in a handler.
1 parent 0d1e2ab commit f420218

9 files changed

+135
-10
lines changed

‎cmd/api-errors.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ const (
139139
ErrObjectExistsAsDirectory
140140
ErrPolicyNesting
141141
ErrInvalidObjectName
142+
ErrInvalidResourceName
142143
ErrServerNotInitialized
143144
// Add new extended error codes here.
144145
// Please open a https://github.com/minio/minio/issues before adding
@@ -588,7 +589,12 @@ var errorCodeResponse = map[APIErrorCode]APIError{
588589
},
589590
ErrInvalidObjectName: {
590591
Code: "XMinioInvalidObjectName",
591-
Description: "Object name contains unsupported characters. Unsupported characters are `^*|\\\"",
592+
Description: "Object name contains unsupported characters.",
593+
HTTPStatusCode: http.StatusBadRequest,
594+
},
595+
ErrInvalidResourceName: {
596+
Code: "XMinioInvalidResourceName",
597+
Description: "Resource name contains bad components such as \"..\" or \".\".",
592598
HTTPStatusCode: http.StatusBadRequest,
593599
},
594600
ErrServerNotInitialized: {

‎cmd/generic-handlers.go

+49
Original file line numberDiff line numberDiff line change
@@ -419,3 +419,52 @@ func (h httpStatsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
419419
// Update http statistics
420420
globalHTTPStats.updateStats(r, ww, durationSecs)
421421
}
422+
423+
// pathValidityHandler validates all the incoming paths for
424+
// any bad components and rejects them.
425+
type pathValidityHandler struct {
426+
handler http.Handler
427+
}
428+
429+
func setPathValidityHandler(h http.Handler) http.Handler {
430+
return pathValidityHandler{handler: h}
431+
}
432+
433+
// Bad path components to be rejected by the path validity handler.
434+
const (
435+
dotdotComponent = ".."
436+
dotComponent = "."
437+
)
438+
439+
// Check if the incoming path has bad path components,
440+
// such as ".." and "."
441+
func hasBadPathComponent(path string) bool {
442+
path = strings.TrimSpace(path)
443+
for _, p := range strings.Split(path, slashSeparator) {
444+
switch strings.TrimSpace(p) {
445+
case dotdotComponent:
446+
return true
447+
case dotComponent:
448+
return true
449+
}
450+
}
451+
return false
452+
}
453+
454+
func (h pathValidityHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
455+
// Check for bad components in URL path.
456+
if hasBadPathComponent(r.URL.Path) {
457+
writeErrorResponse(w, ErrInvalidResourceName, r.URL)
458+
return
459+
}
460+
// Check for bad components in URL query values.
461+
for _, vv := range r.URL.Query() {
462+
for _, v := range vv {
463+
if hasBadPathComponent(v) {
464+
writeErrorResponse(w, ErrInvalidResourceName, r.URL)
465+
return
466+
}
467+
}
468+
}
469+
h.handler.ServeHTTP(w, r)
470+
}

‎cmd/object-api-utils.go

+3
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ func IsValidObjectName(object string) bool {
131131
// IsValidObjectPrefix verifies whether the prefix is a valid object name.
132132
// Its valid to have a empty prefix.
133133
func IsValidObjectPrefix(object string) bool {
134+
if hasBadPathComponent(object) {
135+
return false
136+
}
134137
if len(object) > 1024 {
135138
return false
136139
}

‎cmd/object-api-utils_test.go

+10
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,22 @@ func TestIsValidObjectName(t *testing.T) {
100100
{"contains-|-pipe", true},
101101
{"contains-\"-quote", true},
102102
{"contains-`-tick", true},
103+
{"..test", true},
104+
{".. test", true},
105+
{". test", true},
106+
{".test", true},
103107
{"There are far too many object names, and far too few bucket names!", true},
104108
// cases for which test should fail.
105109
// passing invalid object names.
106110
{"", false},
107111
{"a/b/c/", false},
108112
{"/a/b/c", false},
113+
{"../../etc", false},
114+
{"../../", false},
115+
{"/../../etc", false},
116+
{" ../etc", false},
117+
{"./././", false},
118+
{"./etc", false},
109119
{"contains-\\-backslash", false},
110120
{string([]byte{0xff, 0xfe, 0xfd}), false},
111121
}

‎cmd/object-handlers_test.go

+53-1
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,58 @@ func testAPIGetObjectHandler(obj ObjectLayer, instanceType, bucketName string, a
316316
expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidAccessKeyID), getGetObjectURL("", bucketName, objectName))),
317317
expectedRespStatus: http.StatusForbidden,
318318
},
319+
// Test case - 7.
320+
// Case with bad components in object name.
321+
{
322+
bucketName: bucketName,
323+
objectName: "../../etc",
324+
byteRange: "",
325+
accessKey: credentials.AccessKey,
326+
secretKey: credentials.SecretKey,
327+
328+
expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidObjectName),
329+
getGetObjectURL("", bucketName, "../../etc"))),
330+
expectedRespStatus: http.StatusBadRequest,
331+
},
332+
// Test case - 8.
333+
// Case with strange components but returning error as not found.
334+
{
335+
bucketName: bucketName,
336+
objectName: ". ./. ./etc",
337+
byteRange: "",
338+
accessKey: credentials.AccessKey,
339+
secretKey: credentials.SecretKey,
340+
341+
expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrNoSuchKey),
342+
"/"+bucketName+"/"+". ./. ./etc")),
343+
expectedRespStatus: http.StatusNotFound,
344+
},
345+
// Test case - 9.
346+
// Case with bad components in object name.
347+
{
348+
bucketName: bucketName,
349+
objectName: ". ./../etc",
350+
byteRange: "",
351+
accessKey: credentials.AccessKey,
352+
secretKey: credentials.SecretKey,
353+
354+
expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrInvalidObjectName),
355+
"/"+bucketName+"/"+". ./../etc")),
356+
expectedRespStatus: http.StatusBadRequest,
357+
},
358+
// Test case - 10.
359+
// Case with proper components
360+
{
361+
bucketName: bucketName,
362+
objectName: "etc/path/proper/.../etc",
363+
byteRange: "",
364+
accessKey: credentials.AccessKey,
365+
secretKey: credentials.SecretKey,
366+
367+
expectedContent: encodeResponse(getAPIErrorResponse(getAPIError(ErrNoSuchKey),
368+
getGetObjectURL("", bucketName, "etc/path/proper/.../etc"))),
369+
expectedRespStatus: http.StatusNotFound,
370+
},
319371
}
320372

321373
// Iterating over the cases, fetching the object validating the response.
@@ -346,7 +398,7 @@ func testAPIGetObjectHandler(obj ObjectLayer, instanceType, bucketName string, a
346398
}
347399
// Verify whether the bucket obtained object is same as the one created.
348400
if !bytes.Equal(testCase.expectedContent, actualContent) {
349-
t.Errorf("Test %d: %s: Object content differs from expected value.: %s", i+1, instanceType, string(actualContent))
401+
t.Errorf("Test %d: %s: Object content differs from expected value %s, got %s", i+1, instanceType, testCase.expectedContent, string(actualContent))
350402
}
351403

352404
// Verify response of the V2 signed HTTP request.

‎cmd/routers.go

+2
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ func configureServerHandler(endpoints EndpointList) (http.Handler, error) {
8585

8686
// List of some generic handlers which are applied for all incoming requests.
8787
var handlerFns = []HandlerFunc{
88+
// Validate all the incoming paths.
89+
setPathValidityHandler,
8890
// Network statistics
8991
setHTTPStatsHandler,
9092
// Limits all requests size to a maximum fixed limit

‎cmd/server_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func (s *TestSuiteCommon) TestObjectDir(c *C) {
170170
response, err = client.Do(request)
171171

172172
c.Assert(err, IsNil)
173-
verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters. Unsupported characters are `^*|\\\"", http.StatusBadRequest)
173+
verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters.", http.StatusBadRequest)
174174
}
175175

176176
func (s *TestSuiteCommon) TestBucketSQSNotificationAMQP(c *C) {
@@ -1245,7 +1245,7 @@ func (s *TestSuiteCommon) TestPutObjectLongName(c *C) {
12451245

12461246
response, err = client.Do(request)
12471247
c.Assert(err, IsNil)
1248-
verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters. Unsupported characters are `^*|\\\"", http.StatusBadRequest)
1248+
verifyError(c, response, "XMinioInvalidObjectName", "Object name contains unsupported characters.", http.StatusBadRequest)
12491249
}
12501250

12511251
// TestNotBeAbleToCreateObjectInNonexistentBucket - Validates the error response

‎cmd/storage-errors.go

+3
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ var errFileNotFound = errors.New("file not found")
4848
// errFileNameTooLong - given file name is too long than supported length.
4949
var errFileNameTooLong = errors.New("file name too long")
5050

51+
// errFileComponentInvalid - given file name has invalid components.
52+
var errFileComponentInvalid = errors.New("file name has invalid components")
53+
5154
// errVolumeExists - cannot create same volume again.
5255
var errVolumeExists = errors.New("volume already exists")
5356

‎cmd/test-utils_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ func StartTestServer(t TestErrHandler, instanceType string) TestServer {
318318
// The object Layer will be a temp back used for testing purpose.
319319
func initTestStorageRPCEndPoint(endpoints EndpointList) http.Handler {
320320
// Initialize router.
321-
muxRouter := router.NewRouter()
321+
muxRouter := router.NewRouter().SkipClean(true)
322322
registerStorageRPCRouters(muxRouter, endpoints)
323323
return muxRouter
324324
}
@@ -389,7 +389,7 @@ func StartTestPeersRPCServer(t TestErrHandler, instanceType string) TestServer {
389389
testRPCServer.Obj = objLayer
390390
globalObjLayerMutex.Unlock()
391391

392-
mux := router.NewRouter()
392+
mux := router.NewRouter().SkipClean(true)
393393
// need storage layer for bucket config storage.
394394
registerStorageRPCRouters(mux, endpoints)
395395
// need API layer to send requests, etc.
@@ -2154,7 +2154,7 @@ func registerAPIFunctions(muxRouter *router.Router, objLayer ObjectLayer, apiFun
21542154
func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Handler {
21552155
// initialize a new mux router.
21562156
// goriilla/mux is the library used to register all the routes and handle them.
2157-
muxRouter := router.NewRouter()
2157+
muxRouter := router.NewRouter().SkipClean(true)
21582158
if len(apiFunctions) > 0 {
21592159
// Iterate the list of API functions requested for and register them in mux HTTP handler.
21602160
registerAPIFunctions(muxRouter, objLayer, apiFunctions...)
@@ -2171,15 +2171,15 @@ func initTestWebRPCEndPoint(objLayer ObjectLayer) http.Handler {
21712171
globalObjLayerMutex.Unlock()
21722172

21732173
// Initialize router.
2174-
muxRouter := router.NewRouter()
2174+
muxRouter := router.NewRouter().SkipClean(true)
21752175
registerWebRouter(muxRouter)
21762176
return muxRouter
21772177
}
21782178

21792179
// Initialize browser RPC endpoint.
21802180
func initTestBrowserPeerRPCEndPoint() http.Handler {
21812181
// Initialize router.
2182-
muxRouter := router.NewRouter()
2182+
muxRouter := router.NewRouter().SkipClean(true)
21832183
registerBrowserPeerRPCRouter(muxRouter)
21842184
return muxRouter
21852185
}
@@ -2233,7 +2233,7 @@ func StartTestS3PeerRPCServer(t TestErrHandler) (TestServer, []string) {
22332233
globalObjLayerMutex.Unlock()
22342234

22352235
// Register router on a new mux
2236-
muxRouter := router.NewRouter()
2236+
muxRouter := router.NewRouter().SkipClean(true)
22372237
err = registerS3PeerRPCRouter(muxRouter)
22382238
if err != nil {
22392239
t.Fatalf("%s", err)

0 commit comments

Comments
 (0)