Weak log ouput sanitization (Recurity Gitaly assessment)
https://gitlab.com/gitlab-com/security/issues/21
https://drive.google.com/file/d/1tGmZlTjNxr1eCUKxf9Et_quxrrq5pfTC/view
Details
Location: internal/logsanitizer/url.go
The URL sanitizer, introduced in 7, aims to prevent user credentials within URLs from being logged. The implementation uses the following character class within a Regular Expression to match username and password: [a-z0-9\-._~%!$&'()*+,;=:]
The whole Regular Expression is used with the (?i) flag in order to match case insensitive. However, the matching of the non-alphanumeric characters (\-._~%!$&'()*+,;=:)
is not sufficient to prevent the logging of credentials in cases where the credentials contain characters not defined in that class.
Reproduction Steps
The following short Go program illustrates the issue:
package main
import (
"os"
"fmt"
"regexp"
)
var hostPattern = regexp.MustCompile(`(?i)([a-z][a-z0-9+\-.]*://)([a-z0-
9\-._~%!$&'()*+,;=:]+@)([a-z0-9\-._~%]+|[[a-z0-9\-._~%!$&'()*+,;=:]+])`)
func sanitizeString(str string) string {
return hostPattern.ReplaceAllString(str, "$1[FILTERED]@$3$4")
}
func main() {
fmt.Println(sanitizeString(os.Args[1]))
}
For instance, when run with a to-be-sanitized input of http://user:p^ssword@localhost/
, the sanitizeString
routine does not catch the credential part of the supplied URL:
$ go run main.go 'http://user:p^ssword@localhost/' http://user:p^ssword@localhost/
Recommendation
Instead of utilizing Regular Expressions to attempt to sanitize credentials within URLs in the logging, a more solid approach should be considered. Recurity Labs recommends to transfer credentials (usernames/passwords) within the gRPC communication separately from the URL in another field.