[bug] Crash with version matching / parsing

[jeffmendoza]

Problem is here: https://github.com/guacsec/guac/blob/main/pkg/misc/depversion/depversion.go#L356

This isDependency was added by the deps.dev collector

    "IsDependency": [
      {
        "id": "4360",
        "justification": "dependency data collected via deps.dev",
        "package": {
          "id": "2",
          "type": "npm",
          "namespaces": [
            {
              "id": "6",
              "namespace": "",
              "names": [
                {
                  "id": "476",
                  "name": "readable-stream",
                  "versions": [
                    {
                      "id": "3617",
                      "version": "1.0.34",
                      "qualifiers": [],
                      "subpath": ""
                    }
                  ]
                }
              ]
            }
          ]
        },
        "dependentPackage": {
          "id": "2",
          "type": "npm",
          "namespaces": [
            {
              "id": "6",
              "namespace": "",
              "names": [
                {
                  "id": "774",
                  "name": "string_decoder",
                  "versions": []
                }
              ]
            }
          ]
        },
        "dependencyType": "DIRECT",
        "versionRange": "~0.10.x",
        "origin": "deps.dev",
        "collector": "deps.dev"
      },

Note the "versionRange": "~0.10.x",. The getConstraint() function can handle a version range that starts with ~ or has a wildcard x, but not both.

It then tries to pass the wildcard semver to parseSemver() and that chokes with

panic: runtime error: index out of range [1] with length 0

goroutine 1 [running]:
github.com/guacsec/guac/pkg/misc/depversion.parseSemverHelper(0xc000373cc0, {0xc0004df687, 0x6})
	/home/jeffm/git/guac/pkg/misc/depversion/depversion.go:270 +0xc65

https://github.com/guacsec/guac/blob/main/pkg/misc/depversion/depversion.go#L249 Should check that len(matches) > 0 and return error if not to avoid panic on the index on line 263.

Once this isDep is in your graph, any guacone query vuln that tries to traverse will fail.

[jeffmendoza]

Here is another dep that causes issues

      {
        "id": "439",
        "justification": "dependency data collected via deps.dev",
        "package": {
          "id": "2",
          "type": "pypi",
          "namespaces": [
            {
              "id": "3",
              "namespace": "",
              "names": [
                {
                  "id": "169",
                  "name": "s3transfer",
                  "versions": [
                    {
                      "id": "417",
                      "version": "0.6.1",
                      "qualifiers": [],
                      "subpath": ""
                    }
                  ]
                }
              ]
            }
          ]
        },
        "dependentPackage": {
          "id": "2",
          "type": "pypi",
          "namespaces": [
            {
              "id": "3",
              "namespace": "",
              "names": [
                {
                  "id": "12",
                  "name": "botocore",
                  "versions": []
                }
              ]
            }
          ]
        },
        "dependencyType": "DIRECT",
        "versionRange": "<2.0a.0,>=1.12.36",
        "origin": "deps.dev",
        "collector": "deps.dev"
      },

"<2.0a.0,>=1.12.36" causes fixAlmostSemVer() to be called on https://github.com/guacsec/guac/blob/main/pkg/misc/depversion/depversion.go#L419 with the parameter 2.0a.0, which doesn't match the regex, and the matches[] index on https://github.com/guacsec/guac/blob/main/pkg/misc/depversion/depversion.go#L177 results in panic: runtime error: index out of range [1] with length 0

[nathannaveen]

I am going to start working on this.

[nathannaveen]

@jeffmendoza Could you please tell me what the expected output for <2.0a.0,>=1.12.36 because I don't really understand.

[lumjjb]

in that case it should return and error without panicing and that should be good I think?