Skip to content
Permalink
Browse files Browse the repository at this point in the history
pkg/tool: improve SanitizePath (#5558)
  • Loading branch information
unknwon committed Dec 18, 2018
1 parent 86ada87 commit ff93d9d
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 3 deletions.
2 changes: 1 addition & 1 deletion gogs.go
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/gogs/gogs/pkg/setting"
)

const APP_VER = "0.11.81.1217"
const APP_VER = "0.11.82.1218"

func init() {
setting.AppVer = APP_VER
Expand Down
4 changes: 3 additions & 1 deletion pkg/tool/path.go
Expand Up @@ -17,5 +17,7 @@ func IsSameSiteURLPath(url string) bool {

// SanitizePath sanitizes user-defined file paths to prevent remote code execution.
func SanitizePath(path string) string {
return strings.TrimLeft(path, "./")
path = strings.TrimLeft(path, "/")
path = strings.Replace(path, "../", "", -1)

This comment has been minimized.

Copy link
@seadra

seadra Dec 20, 2018

How about ".../", "..../" and so on? Or, isn't it safer to fail when path.IsAbs() fails?

This comment has been minimized.

Copy link
@unknwon

unknwon Dec 20, 2018

Author Member

Hi, can you prove .../ and ..../ actually cause problem? And path passed in is not guaranteed to be a abs path, so it is not suitable.

return path
}
1 change: 1 addition & 0 deletions pkg/tool/path_test.go
Expand Up @@ -38,6 +38,7 @@ func Test_SanitizePath(t *testing.T) {
expect string
}{
{"../../../../../../../../../data/gogs/data/sessions/a/9/a9f0ab6c3ef63dd8", "data/gogs/data/sessions/a/9/a9f0ab6c3ef63dd8"},
{"data/gogs/../../../../../../../../../data/sessions/a/9/a9f0ab6c3ef63dd8", "data/gogs/data/sessions/a/9/a9f0ab6c3ef63dd8"},

{"data/sessions/a/9/a9f0ab6c3ef63dd8", "data/sessions/a/9/a9f0ab6c3ef63dd8"},
}
Expand Down
2 changes: 1 addition & 1 deletion templates/.VERSION
@@ -1 +1 @@
0.11.81.1217
0.11.82.1218

0 comments on commit ff93d9d

Please sign in to comment.