Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function leafInfo.match() use path.join() to deal with wildcardValues, which may lead to cross directory risk. #4961

Closed
runner361 opened this issue May 23, 2022 · 4 comments · Fixed by #5025

Comments

@runner361
Copy link
Contributor

runner361 commented May 23, 2022

Function leafInfo.match() use path.join() to deal with wildcardValues, which may lead to cross directory risk.

  • poc1: route end with *.* can use ../ to cross directory and set evil value for :path .
    ctx.Input.SetParam(":path", path.Join(path.Join(wildcardValues[:len(wildcardValues)-1]...), strs[0]))

    ctx.Input.SetParam(":path", path.Join(path.Join(wildcardValues[index:len(wildcardValues)-1]...), strs[0]))

    For route /book1/:name/fixPath1/*.* , urls below can match, and set :path=evil
    /book1/name1/fixPath1/mybook/../evil.txt =>:name=name1
    /book1/name1/fixPath1/mybook/../././evil.txt =>:name=name1
    /book1/name1/fixPath1/mybook/../././////evil.txt =>:name=name1
    /book1/../fixPath1/mybook/../././////evil.txt =>:name=..
    /book1/./fixPath1/mybook/../././////evil.txt =>:name=.
    image
//Test code as below:
web.Router("/book1/:name/fixPath1/*.*", &controllers.BookController{}, "get:SearchByName")
func (b BookController) SearchByName() {
	fmt.Println(":path=" + b.Ctx.Input.Param(":path"))
	fmt.Println(":name=" + b.Ctx.Input.Param(":name"))
	b.Data["json"] = "OK"
	b.ServeJSON()
}
  • poc2: regex route can use ../ to cross directory and replace wildcard with evil value
    if !leaf.regexps.MatchString(path.Join(wildcardValues...)) {

    For regex route /book2/:type:string/fixPath1/:name,urls below can match and value of :type :name can be replaced with evil value.
    /book2/type1/fixPath1/name1/../../evilType/evilName => :type=evilType :name=evilName
    /book2/type1/fixPath1/name1/../../././evilType/evilName => :type=evilType :name=evilName
    /book2/type1/fixPath1/name1/../../././////evilType/evilName=> :type=evilType :name=evilName
    image
//Test code as below:
web.Router("/book2/:type:string/fixPath1/:name", &controllers.BookController{}, "get:SearchByType")
func (b BookController) SearchByType() {
	fmt.Println(":name=" + b.Ctx.Input.Param(":name"))
	fmt.Println(":type=" + b.Ctx.Input.Param(":type"))
	b.Data["json"] = "OK"
	b.ServeJSON()
}
@flycash
Copy link
Collaborator

flycash commented May 23, 2022

yes, can you help to fix it?

@runner361
Copy link
Contributor Author

runner361 commented May 24, 2022

yes, can you help to fix it?

Ok, I will try. Url need to be normalized before route, and path.join() should be replaced with strings.join().

@flycash
Copy link
Collaborator

flycash commented May 24, 2022

yes, can you help to fix it?

Ok, I will try. Url need to be normalized before route, and path.join() should be replaced with strings.join().

But if you use strings.join, you need to handle the case that different platform use different symbol as path separator

@runner361
Copy link
Contributor Author

But if you use strings.join, you need to handle the case that different platform use different symbol as path separator

Url only use "/" as split char, no need to care about different platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants