Enhancement to Reporting (JSON)

[streichsbaer]

First of all, I'd like to say thank you for the hard work that went into sobelow.

I've looked into making sobelow accessible through Guardrails.

However, there are some challenges with processing the JSON output format as described below. I would love to fund work on this issue using gitcoin.

1. The findings are not consistently reporting file names.

Only some of the modules report the offending file.

For example:

  "type": "Config.HTTPS: HTTPS Not Enabled"

vs

  "file": "config/config.exs - line 23",
  "key": "secret_key",
  "type": "Config.Secrets: Hardcoded Secret"

vs

  "file": "lib/broadway/server.ex",
  "function": "build_consumer_supervisor_spec:381",
  "type": "DOS.BinToAtom: Unsafe atom interpolation",
  "variable": "name_prefix and key"

2. Some filenames contain the line number, some don't.

There are inconsistencies in how the filenames are reported.

  "file": "config/config.exs - line 23",

vs

  "file": "/lib/broadway/server.ex",

3. The line numbers are not pointing to the offending line.

This has been reported previously (#37), but the decision to point to the function signature makes it difficult to point developers to the right line.

For example:

  "file": "config/config.exs - line 23",

points to:

config :real_world, RealWorldWeb.Guardian,

instead of:

  secret_key: "MDLMflIpKod5YCnkdiY7C4E3ki2rgcAAMwfBl0+vyC5uqJNgoibfQmAh7J3uZWVK",

Conclusion

To conclude, I would suggest that all findings:

• include a consistent filename
• include a lineNumber that points to the offending line of code (can be 0 if a config setting is missing)

Note that the function field can remain as is.

@GriffinMB and team I would appreciate your thoughts on that.

Thanks!

[GriffinMB]

Hey @streichsbaer!

Thanks for opening this issue. I agree that the reporting could use consistency improvements. I also agree that the line details aren't ideal for the JSON output, where you don't get highlighted code to pinpoint the vulnerability. This may take a little bit of time as I consider exactly how I'd like to approach these updates, but this is definitely something I'd like to address. That said, I don't expect that this will take too long to resolve.

Thanks again for opening the issue!

[streichsbaer]

Thanks @GriffinMB, great to hear it!

As mentioned, I'm very happy to support it via gitcoin or other means. WDTY?

[GriffinMB]

No financial support needed, but I appreciate the offer :)

[streichsbaer]

Alright, thanks a lot! Do ping me if you think there is anything I can do to support/help.

[GriffinMB]

Hey @streichsbaer! I've pushed some updates to master that improve output consistency. All JSON output now contains the keys, "type", "line" and "file". The line number points directly to the offending issue, and defaults to 0 in instances where a line number may not make sense. Instances like "Missing CSRF Protections" behave a little differently, and "line" will point to the pipeline where the configuration is missing.

You can install the master version with mix archive.install github nccgroup/sobelow to test it out. It's a fairly large change, so please let me know if you run into any issues!

I will leave this open for now, and will aim to release a new version early next week if there are no reported problems.

Thanks again!

[streichsbaer]

Awesome, thanks @GriffinMB, I'll start testing this week and will let you know if I run into any issues.

[GriffinMB]

Hey @streichsbaer! I intend on pushing the update as early as tomorrow if you don't have any feedback or issues.

[GriffinMB]

This has been pushed to hex.pm! Thanks for opening the issue :)

[streichsbaer]

Hello @GriffinMB, I've tested it against 5 projects with 14 total issues and it all looked fine! Will advise if anything else comes up.

Thanks again for the excellent work and fast turnaround time!