Skip to content

Commit

Permalink
Stop bundling GitHub and GitLab commit URLs in with Bitbucket (google…
Browse files Browse the repository at this point in the history
…#1699)

This was resulting in an erroneous false match and an invalid fix commit
hash being extracted.
  • Loading branch information
andrewpollock authored Oct 6, 2023
1 parent 8ea0d15 commit 111b7d7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
17 changes: 13 additions & 4 deletions vulnfeeds/cves/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -681,15 +681,24 @@ func Commit(u string) (string, error) {
// https://github.com/MariaDB/server/commit/b1351c15946349f9daa7e5297fb2ac6f3139e4a8
// https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/b05bb61f454eeb8a85164c8a31510aeb9d79129c
// https://gitlab.com/qemu-project/qemu/-/commit/4367a20cc4

parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/")
directory, possibleCommitHash := path.Split(parsedURL.Path)
if strings.HasSuffix(directory, "commit/") {
return strings.TrimSuffix(possibleCommitHash, ".patch"), nil
}

// and Bitbucket.org commit URLs are similiar yet slightly different:
// https://bitbucket.org/openpyxl/openpyxl/commits/3b4905f428e1
//
// Some bitbucket.org commit URLs have been observed in the wild with a trailing /, which will
// change the behaviour of path.Split(), so normalize the path to be tolerant of this.
parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/")
directory, possibleCommitHash := path.Split(parsedURL.Path)
if strings.HasSuffix(directory, "commit/") || strings.HasSuffix(directory, "commits/") {
return strings.TrimSuffix(possibleCommitHash, ".patch"), nil
if parsedURL.Host == "bitbucket.org" {
parsedURL.Path = strings.TrimSuffix(parsedURL.Path, "/")
directory, possibleCommitHash := path.Split(parsedURL.Path)
if strings.HasSuffix(directory, "commits/") {
return possibleCommitHash, nil
}
}

// TODO(apollock): add support for resolving a GitHub PR to a commit hash
Expand Down
12 changes: 12 additions & 0 deletions vulnfeeds/cves/versions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,12 @@ func TestExtractGitCommit(t *testing.T) {
Fixed: "cd4e934d0527e5010e373e7fed54ef5daefba2f5",
},
},
{
description: "Undesired GitHub commit URL", // TODO(apollock): be able to parse this a a LastAffected commit
inputLink: "https://github.com/Budibase/budibase/commits/develop?after=93d6939466aec192043d8ac842e754f65fdf2e8a+594\u0026branch=develop\u0026qualified_name=refs%2Fheads%2Fdevelop",
inputCommitType: Fixed,
expectFailure: true,
},
{
description: "Valid GitHub commit URL with .patch extension",
inputLink: "https://github.com/pimcore/customer-data-framework/commit/e3f333391582d9309115e6b94e875367d0ea7163.patch",
Expand All @@ -425,6 +431,12 @@ func TestExtractGitCommit(t *testing.T) {
Fixed: "e3f333391582d9309115e6b94e875367d0ea7163",
},
},
{
description: "Undesired GitHub PR commit URL",
inputLink: "https://github.com/OpenZeppelin/cairo-contracts/pull/542/commits/6d4cb750478fca2fd916f73297632f899aca9299",
inputCommitType: Fixed,
expectFailure: true,
},
{
description: "Valid GitLab commit URL",
inputLink: "https://gitlab.freedesktop.org/virgl/virglrenderer/-/commit/b05bb61f454eeb8a85164c8a31510aeb9d79129c",
Expand Down

0 comments on commit 111b7d7

Please sign in to comment.