logpresso / CVE-2021-44228-Scanner

Vulnerability scanner and mitigation patch for Log4j2 CVE-2021-44228
Apache License 2.0
850 stars 174 forks source link

Restriction preventing use of both --report-csv and --report-json #210

Closed sds83uk closed 2 years ago

sds83uk commented 2 years ago

Hi,

I drafted the following a few days ago but at that time a problem with my account meant that I was unable to add as a comment to a previous issue. As this now covers two closed issues (references added) and has a separate request I hope this the an appropriate way to raise this.

Original example (#174, resolved in version 2.4.1)

In version 2.4.0 there is an output which explains what happened here: Command: log4j2-scan.exe --report-json --report-path out.json c:\ Output (v.2.4.0): Cannot write report file. File already exists: c:\out.json i.e. the CSV was written first and the JSON file couldn't be created

Example 2 (#203, resolved in version 2.6.1)

I mention this because in version 2.4.0 --report-json --report-dir Reports creates both JSON and CSV (due to unique names): Command: log4j2-scan.exe --report-json --report-dir Reports C:\

Whereas, because --report-dir still implies --report-csv, version 2.4.1 advises: Output (v2.4.1): Error: Cannot use both --report-csv and --report-json options. Choose one.

Changing tack slightly... The reason for raising this issue...

Example 3 (this issue)

Whilst testing version 2.4.0 with the following command the output was beautiful, wonderful work! Command: log4j2-scan.exe --all-drives --scan-log4j1 --scan-logback --scan-zip --report-csv --report-json --no-empty-report >> %COMPUTERNAME%_log4j2_scan_report.txt

Testing again with version 2.4.1, the same error is presented: Output (v2.4.1): Error: Cannot use both --report-csv and --report-json options. Choose one.

Request

Would it be possible/appropriate to remove this check? https://github.com/logpresso/CVE-2021-44228-Scanner/blob/3d82e03432188085d73e43356cefa515a6cf4245/src/main/java/com/logpresso/scanner/Configuration.java#L323-L325

Because due to the other alterations [in version 2.4.1 and 2.6.1]:

https://github.com/logpresso/CVE-2021-44228-Scanner/blob/3d82e03432188085d73e43356cefa515a6cf4245/src/main/java/com/logpresso/scanner/Configuration.java#L327-L332

This would allow us to continue to create both types of reports whilst we determine how we plan to consume the data.

Thank you for your time and consideration.

xeraph commented 2 years ago

@sds83uk Why do you need both csv and json report file which has the same data?

sds83uk commented 2 years ago

Unfortunately we are still very much in the investigation stage. The CSV format will be more comfortable for some teams to review. The detailed information (and errors) presented in the JSON file is great for our technical teams.

xeraph commented 2 years ago

@sds83uk Since csv and json shares same --report-path and --report-dir, you should choose one. (For example, if you specify --report-path, what does it mean? csv path? or json path?) If some teams prefer CSV format, you can convert json to csv files using Logpresso Mini (another freeware).

Download windows or linux binary from here https://github.com/logpresso/community

Use this command with uploaded query.txt

logpresso.exe -f query.txt

Then, Logpresso Mini will merge all JSON report files into single CSV file log4j2_scan_all_reports.csv

Query:

 textfile erex="^}$" log4j2_scan_report*.json 
 | parsejson 
 | parsemap field=summary overlay=t | explode files 
 | parsemap field=files overlay=t | explode reports 
 | parsemap field=reports overlay=t
 | fields hostname, path, entry, product, version, cve, status, fixed, detected_at
 | outputcsv log4j2_scan_all_reports.csv hostname, path, entry, product, version, cve, status, fixed, detected_at
sds83uk commented 2 years ago

My apologies, I see it now! It was late when I wrote my comment and I didn't put it under scrutiny again in the light of day.

Could the snippet I suggested removing be changed to only apply to --report-path (i.e. if the the following parameters are specified): --report-csv --report-json --report-path?

I understand and would be okay with both types of reports being created in the same folder if the following parameters were used: --report-csv --report-json or --report-csv --report-json --report-dir Reports

If needed, a caller/wrapper script can be modified to move these to alternative locations (post execution).

Having said that, I also recognise that someone else may have a different requirement to store the files in separate folders during execution (rather than running twice) and will certainly look into the tool you have suggested to see what we can do with that.

Thank you for your continued contributions to the community!

xeraph commented 2 years ago

@sds83uk A bit complicated but it makes sense. I'll fix it.

xeraph commented 2 years ago

@sds83uk Fixed in v2.6.2. Would you test it?

sds83uk commented 2 years ago

Thank you! This is now working as requested:

Create named report files

  1. --report-path Test1.csv OUTPUT: created CSV (as --report-path "Implies --report-csv.")

  2. --report-path Test2.csv --report-csv OUTPUT: created CSV

  3. --report-path Test3.csv --report-csv --report-json OUTPUT: Cannot use both --report-csv and --report-json options if --report-path is specified. Choose one.

  4. --report-path Test4.json --report-json OUTPUT: created JSON

Note: Above tests fail if file already exists, i.e. "Error: File already exists - ...\Test1.csv"

Create timestamped report files in specified output directory:

_File name = log4j2_scan_report_yyyyMMddHHmmss

  1. --report-dir OutputDir OUTPUT: created CSV (as --report-dir "Implies --report-csv.")

  2. --report-dir OutputDir --report-csv OUTPUT: created CSV

  3. --report-dir OutputDir --report-csv --report-json OUTPUT: created CSV and JSON

  4. --report-dir OutputDir --report-json OUTPUT: created JSON

  5. --report-csv OUTPUT: created CSV

  6. --report-json OUTPUT: created JSON

xeraph commented 2 years ago

@sds83uk Thank you for detail test report!