From f8e774393c30430714adae3f306811ed52db51e4 Mon Sep 17 00:00:00 2001 From: Konrad Wojas Date: Fri, 1 May 2020 22:11:37 +0800 Subject: [PATCH] Stricter path sanitization Goji routes incoming requests without first URL decoding the path, so '%2F' in a URL will not be decoded to a '/' before routing. But by the time that we perform the path checks for private urls on r.URL.Path, these characters have been decoded. As a consequence, a user 'foo' could use 'foo%2Fbar' as the repo name. The private repo check would see that the path starts with 'foo/' and allow it, and rest-server would happily create a 'foo/bar' repo. Other more harmful variants are possible. To resolve this issue, we now reject any name part that contains a '/'. Additionally, we immediately reject a few other characters that are disallowed under some operating systems or filesystems. --- handlers.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/handlers.go b/handlers.go index d0c665f..7809e92 100644 --- a/handlers.go +++ b/handlers.go @@ -44,11 +44,18 @@ func (s *Server) isHashed(dir string) bool { } func valid(name string) bool { - // taken from net/http.Dir + // Based on net/http.Dir if strings.Contains(name, "\x00") { return false } + // Path characters that are disallowed or unsafe under some operating systems + // are not allowed here. + // The most important one here is '/', since Goji does not decode '%2F' to '/' + // during routing, so we can end up with a '/' in the name here. + if strings.ContainsAny(name, "/\\:*?\"<>|") { + return false + } if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) { return false }