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

More human readable output #46

Merged
merged 2 commits into from
Dec 21, 2021
Merged

Conversation

hpryce
Copy link
Contributor

@hpryce hpryce commented Dec 21, 2021

This adds much more explanation for when being invoked directly by a user, with the individual match hits coming up as info lines and then the overall summary as a match line.

Colours are used to try and make it easy to see the most important parts of the output.

Intended to address #41

@hpryce hpryce changed the base branch from develop to more_reporting_output December 21, 2021 17:45
Copy link
Contributor

@nmiyake nmiyake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content looks good, but concerns around the %s format string printing directly in some calls.

@@ -313,6 +320,28 @@ func (i *Log4jIdentifier) lookForMatchInTar(ctx context.Context, getTarReader ar
return archiveResult, versions, nil
}

func (i *Log4jIdentifier) printDetailedHashFinding(path string, finding Finding) {
if finding&ClassFileMd5 > 0 {
i.printInfoFinding("Found JndiManager class that was an exact md5 match for a known version at %s", path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think this outputs what you want -- as currently written, I think this will output a literal %s since printInfoFinding takes message and location but prints both literally.

Either printInfoFinding should take a format string or the calls should render directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the case as the colour library takes formatted strings and outputs e.g.:

[INFO] Found finding in what appeared to be an obfuscated jar at examples/obfuscated/2.14.1-aaaagb.jar

} else {
output = fmt.Sprintf(cveMessage+" in file %s. log4j versions: %s. Reasons: %s", path, strings.Join(versions, ", "), strings.Join(readableReasons, ", "))
_, _ = fmt.Fprintln(r.OutputWriter, color.YellowString("[MATCH] "+cveMessage+" in file %s. log4j versions: %s. Reasons: %s", path, strings.Join(versions, ", "), strings.Join(readableReasons, ", ")))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the above, this is using %s but in a Fprintln (rather than Fprintf), so think that %s will print literally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above - I think the colour library is doing formatting and so we're getting the formatted strings printed out.

@hpryce hpryce changed the base branch from more_reporting_output to develop December 21, 2021 18:35
@hpryce
Copy link
Contributor Author

hpryce commented Dec 21, 2021

Example running of latest version:

hpryce25-mac:log4j-sniffer hpryce$ ./out/build/log4j-sniffer/0.8.0-10-g9a184e2.dirty/darwin-amd64/log4j-sniffer crawl examples/single_bad_version/
[INFO] Found JndiManager class that was an exact md5 match for a known version at org/apache/logging/log4j/core/net/JndiManager.class
[INFO] Found JndiLookup class in the log4j package at org/apache/logging/log4j/core/lookup/JndiLookup.class
[MATCH] CVE-2021-45046 and CVE-2021-45105 detected in file examples/single_bad_version/log4j-core-2.14.1.jar. log4j versions: 2.14.0-2.14.1, 2.14.1. Reasons: JndiLookup class and package name matched, jar name matched, JndiManager class and package name matched, class file MD5 matched
Files affected by CVE-2021-45046 or CVE-2021-45105 detected: 1 file(s) impacted by CVE-2021-45046 or CVE-2021-45105
1 total files scanned, skipped 0 paths due to permission denied errors, encountered 0 errors processing paths

Copy link
Contributor

@nmiyake nmiyake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it -- wasn't aware that the color library applied formatting itself as well. Look good.

@bulldozer-bot bulldozer-bot bot merged commit 2cfd78d into develop Dec 21, 2021
@bulldozer-bot bulldozer-bot bot deleted the more_human_readable_output branch December 21, 2021 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants