Report individual findings rather than top-level file #81
Conversation
Generate changelog in
|
glynternet
changed the title
| // we are in an archive and so can assume separator is '/' | ||
| jarVersion, jarNameMatch = pathMatchesLog4JVersion(path) | ||
| jarFinding = JarNameInsideArchive | ||
| if jarNameMatch { |
There was a problem hiding this comment.
This behaviour is carried over from previous implementation.
@hpryce do we want to log at info level when we match any log4j-core jar name or should we only do it when it is a vulnerable version?
There was a problem hiding this comment.
Perhaps controversial, but I think we should drop vulnerable versions from identify.go entirely. We should just find everything here, then filter out in the reporting if it's not vulnerable. This probably means we need a new found but not vulnerable log line in the report.go for the human friendly output.
glynternet
force-pushed
the
gh/report-individual-findings
branch
5 times, most recently
from
180e61f
to
bcd0f74
Compare
|
@glynternet haven't looked at this implementation closely, but if it basically just sets the output path to something of the form Is there another field (like "details") where this would make more sense? I think it would be more consistent/understandable if the "path" always mapped to a file on disk. If we want further information like a path within an archive, perhaps that would make sense as an additional metadata field instead? |
|
This is a very good point @nmiyake, and one I hadn't included when I sketched this out with @glynternet. I think, in the name of avoiding needless breaks, we should add this as another field on the json object. The human readable output I think makes sense to use the paths as done here, and the file path listing should just list the top level path. |
|
Yup, very valid point and sloppy on my part for missing this. |
glynternet
force-pushed
the
gh/report-individual-findings
branch
from
bcd0f74
to
cb996a4
Compare
|
This looks good - the comment thread discussion around logging/handling of non-vulnerable versions is going to be handled in a follow up PR. This now also keeps the json format the same. |
This PR changes the way vulnerabilities are reported. Each finding is now reported individually, rather than reporting and aggregation of all findings with only the top-level file on disk. For example, a vulnerable jar nested inside an archive will now be reported with the vulnerability findings, rather than reporting on the archive with an aggregation of all findings from within it. The path reported with a vulnerability finding is the full path to a finding with archive layers delimited by a "!". i.e. /path/to/archive!path/to/finding.jar shows that an archive at /path/to/archive contained a vulnerable jar at path/to/finding.jar within it.
glynternet
force-pushed
the
gh/report-individual-findings
branch
from
82c40ce
to
1b7f0f9
Compare
|
👍 |
|
Released 1.7.0 |
Closes: #72
This PR changes the way vulnerabilities are reported when not in
--file-path-onlymode.Each finding is now reported individually, rather than reporting
and aggregation of all findings with only the top-level file on
disk.
For example, a vulnerable jar nested inside an archive will now be
reported with the vulnerability findings, rather than reporting on
the archive with an aggregation of all findings from within it.
Multiple vulnerable jars found within an archive will be reported
separately.
The path reported with a vulnerability finding is the full path
to a finding with archive layers delimited by a "!".
i.e. /path/to/archive!path/to/finding.jar shows that an archive
at /path/to/archive contained a vulnerable jar at
path/to/finding.jar within it.
To do this we now accept a
HandleFindingFuncinto theLog4jIdentifier, which is called for every finding encountered whilst crawling.Reporter.Collecthas also been renamed toReportto reflect better the behaviour that it has.