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

Fix regex denial-of-service #27

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix for security denial-of-service issue
Check length of URL and limit to 8000 bytes

Also added a unit test to check
  • Loading branch information
mojotx committed Feb 11, 2024

Verified

This commit was signed with the committer’s verified signature. The key has expired.
mojotx Michael Jarvis
commit 197863a2c936bda81a184404c5bab2499db20e3e
11 changes: 7 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
default: test lint
LINT=golangci-lint run

test:
go test -v ./...
go test -covermode=count -coverprofile=profile.cov .

lint:
gometalinter ./...
$(LINT) ./...

install:
go get -d -v ./... && go build -v ./...
gometalinter --install --update
$(LINT) ./...

deps:
go get github.com/alecthomas/gometalinter
go get golang.org/x/tools/cmd/cover
# binary will be $(go env GOPATH)/bin/golangci-lint
go mod download -x
go mod verify
go mod tidy -v
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
module github.com/whilp/git-urls
module github.com/mojotx/git-urls

go 1.13

require github.com/stretchr/testify v1.8.4
17 changes: 17 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
11 changes: 11 additions & 0 deletions urls.go
Original file line number Diff line number Diff line change
@@ -48,6 +48,8 @@ var (
)
)

const maxUrlLen = 8000

// Parser converts a string into a URL.
type Parser func(string) (*url.URL, error)

@@ -90,6 +92,15 @@ func ParseTransport(rawurl string) (*url.URL, error) {
// ParseScp parses rawurl into a URL object. The rawurl must be
// an SCP-like URL, otherwise ParseScp returns an error.
func ParseScp(rawurl string) (*url.URL, error) {

// Did some research on https://stackoverflow.com/q/417142/31319
// and for best results, URLs should not be more than about 2000
// bytes. Some sites support up to 8000 bytes but that isn't
// widespread yet. We will try using 8000 first.
if len(rawurl) >= maxUrlLen {
return nil, fmt.Errorf("URL too long (%d >= %d)", len(rawurl), maxUrlLen)
}

match := scpSyntax.FindAllStringSubmatch(rawurl, -1)
if len(match) == 0 {
return nil, fmt.Errorf("no scp URL found in %q", rawurl)
46 changes: 32 additions & 14 deletions urls_test.go
Original file line number Diff line number Diff line change
@@ -6,6 +6,8 @@ import (
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

var tests []*Test
@@ -220,20 +222,36 @@ func TestParse(t *testing.T) {
}
}

func TestRegex(t *testing.T) {
var payload = strings.Repeat("////", 19000000) //payload used, the number can be tweaked to cause 7 second delay
malicious_url := "6en6ar@-:0////" + payload + `\`
func runTimingTest(t *testing.T, url string, shouldError bool) {
begin := time.Now()
//u, err := giturls.ParseScp("remote_username@10.10.0.2:/remote/directory")// normal git url
u, err := ParseScp(malicious_url)
// _, err := ParseScp(malicious_url)
if err != nil {
t.Errorf("unable to call ParseScp: %s", err.Error())
}
t.Logf("[ + ] Url --> %+v", u.Host)
elapse := time.Since(begin)
t.Logf("Function took %+v", elapse)
if elapse > time.Second*5 {
t.Errorf("regex took %+v seconds", elapse)

_, err := ParseScp(url)
if shouldError {
assert.Errorf(t, err, "len of %d should trigger error", len(url))
} else {
if t == nil {
panic("t is nil")
}
assert.Nilf(t, err, "unexpected error: %v", err)
}
elapsed := time.Since(begin)
t.Logf("url len is %d, function took %+v", len(url), elapsed)
}

// TestRegex tests to see if we have an excessively long URL
func TestRegex(t *testing.T) {

// First case is 7909 bytes which4yy should still be fast
long_url := `https://=` + strings.Repeat(`/`, 7900)
runTimingTest(t, long_url, false)

// Second case is 20,000 bytes which should be too slow
long_url = `https://=` + strings.Repeat(`/`, 190000000)
runTimingTest(t, long_url, true)

goodURL := `https://stackoverflow.com/q/417142/31319`
runTimingTest(t, goodURL, false)

goodURL = `https://kinesis-ergo.com/wp-content/uploads/Advantage360-SmartSet-KB360-Users-Manual-v10-12-22.pdf`
runTimingTest(t, goodURL, false)
}