boostsecurityio / boostsecurityio/poutine
Bug: Incorrect version check in known vulnerability in build component?
- Dominant language
- Go
- Stars
- 513
- Forks
- 40
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the bug**
A clear and concise description of what the bug is.
I believe there is a bug here: https://github.com/boostsecurityio/poutine/blob/f8df65b916cce85a4a8f025af987549dd0cb9e32/opa/builtins.go#L78
Or at least an overabundance of caution that will lead to too many false positives. The issue is that people often declare actions like: `action@v4`. This means "use the latest version of action where the major version is '4'". However, the logic on the above line, assumes that `action@v4` actually means action v4.0.0. This results in components being reported as vulnerable when they aren't and often have not been for a very long time. One can argue about the pros and cons of not hard pinning an action version, but it is incontrovertible that this is a very common practice to pin, especially to a major version.
I think the proper fix, is to detect this case (probably major version is enough, but someone could theoretically pin to a minor version). And if this exists, and there is a fixed version of the vulnerable component that matches down to the pinned version, then it isn't vulnerable. Probably the simplest way is to pad out the semantic version with an absurdly high value so in the above example "v4" becomes "4.9999999999.9999999999" and "v4.1" becomes "4.1.9999999999". Or you could parse out all the to values in vulnerable ranges and left match against them: regex match '^4\..*' against
**Expected behavior**
We don't get alerts about vulnerable components when we aren't hard-pinning and there's a fixed version.
**Temp Patch**
FWIW, I have made the following changes locally at the above location and "fixed" the issue in my local copy, though I don't know that this is the correct fix for the project as it makes some potential assumptions about the version string and the total number of versions
```golang
// import "strings" at the top
parts := strings.Split(versionStr, ".")
switch len(parts) {
case 1:
versionStr = versionStr + ".999999999.999999999"
case 2:
versionStr = versionStr + ".999999999"
}
// next line is: semver, err := version.NewVersion(versionStr)
```
Contributor guide
Research direction
Start in opa/builtins.go at line 78 and inspect how action versions are parsed and compared with vulnerable and fixed versions. Reproduce the action@v4 and action@v4.1 cases described in the issue, then verify that fixed components no longer produce alerts while genuinely vulnerable pinned versions still do.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- github-actions, go
- Domain
- devops, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100