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

VulnerableCode returns no findings for Go packages #9298

Open
wkl3nk opened this issue Oct 17, 2024 · 2 comments · May be fixed by #9299
Open

VulnerableCode returns no findings for Go packages #9298

wkl3nk opened this issue Oct 17, 2024 · 2 comments · May be fixed by #9299
Assignees
Labels
advisor About the advisor tool bug Issues that are considered to be bugs

Comments

@wkl3nk
Copy link
Contributor

wkl3nk commented Oct 17, 2024

Describe the bug

I am scanning a project with ModGo ORT package manager.
One dependency is quic-go, release 0.40.0 , which definitely has vulnerability findings in the VulnerableCode database.
Unfortunately, no security vulnerabilites are found in the Advisor phase, using VulnerableCode.

Detailed analysis

ORT uses a "bulk search", sending a POST request to the VulnerableCode API. This request has a list of Package URLs (purls) of the dependencies like quick-go.
The purl for quick-go looks as follows:
pkg:golang/github.com%2Fquic-go%[email protected]

So, at some places we see the "/", in other places it is percent-encoded to "%2F".
I don't want to open discussions if this is valid or not, but there is a point in saying "the / character is used to separate the parts that build up a purl, and for this reason, if a namespace or name of a dependency contain /, then they need to be percent-encoded".

When I (in code) add an additional step that replaces all occcurrences of "%2F" with "/" before I feed it into VulnerableCode, then the VulnerableCode API is happy and returns the expected vulnerability records.

Environment

ORT 34.0.0

Additional context

I will prepare a PR to deal with the issue in a pragmatic way. Stay tuned.

@wkl3nk wkl3nk added bug Issues that are considered to be bugs to triage Issues that need triaging labels Oct 17, 2024
@sschuberth sschuberth added advisor About the advisor tool and removed to triage Issues that need triaging labels Oct 17, 2024
@sschuberth
Copy link
Member

sschuberth commented Oct 17, 2024

I don't want to open discussions if this is valid or not, but there is a point in saying "the / character is used to separate the parts that build up a purl, and for this reason, if a namespace or name of a dependency contain /, then they need to be percent-encoded".

Agreed, also regarding the topic that if VulnerableCode would be using a wrong encoding, we'd need to use a wrong encoding as well to make the lookup work.

I will prepare a PR to deal with the issue in a pragmatic way. Stay tuned.

Great! Please also add a test for this, so we'd risk swallowing vulnerabilities in case we do some PURL refactoring again.

wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 17, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.
The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.
This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298

Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 17, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.
The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.
This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298

Signed-off-by: Wolfgang Klenk <[email protected]>
@wkl3nk
Copy link
Contributor Author

wkl3nk commented Oct 17, 2024

Cross-linking the issue ticket in VulnerableCode: aboutcode-org/vulnerablecode#1620

wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 17, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.

The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.

This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298

Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 17, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.

The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.

This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298.

Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 18, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.

The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.

This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298.

Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 18, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.

The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.

This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298.

Signed-off-by: Wolfgang Klenk <[email protected]>
wkl3nk added a commit to boschglobal/oss-review-toolkit that referenced this issue Oct 18, 2024
For Go packages, both the namespace and name may contain path
segments separated by a "/" character. The purl specification
requires these "/" characters to be percent-encoded in the
namespace and name components of a purl.

The VulnerableCode bulk-search API is unable to handle these
percent-encoded "/" characters, resulting in no vulnerability
records being returned.

This bugfix decodes any percent-encoded "/" characters just
before making the VulnerableCode query to ensure proper
functionality.

Fixes oss-review-toolkit#9298.

Signed-off-by: Wolfgang Klenk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advisor About the advisor tool bug Issues that are considered to be bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants