freedomofpress / securedrop-client

a Qt-based GUI for SecureDrop journalists 📰🗞️
GNU Affero General Public License v3.0
39 stars 37 forks source link

Add annotations for data sources and sinks and further static analysis #385

Open emkll opened 5 years ago

emkll commented 5 years ago

Pyre [0] allows for further static analysis to ensure untrusted and/or unsanitized input never makes it it's way through to sensitive functions.

We should add annotations for sources, sanitizers and sinks across the codebase in order for this static analysis to run.

[0] : https://pyre-check.org/docs/static-analysis-post-processor.html

redshiftzero commented 5 years ago

for the interested observer, check out the talk from Pycon 2019 for background on this

kushaldas commented 4 years ago

pyre static analysis works in example code, but, I could not find any other project on Github using this. i tried to add the minimal along with adding eval calls, which should cause red flags. But, could not do it easily. I have pinged my friends to get more direct connection to the upstream community, but, nothing mentions in the documentation as a community place, I personally think we should wait before starting to use it.

eloquence commented 3 years ago

We still agree that static analysis would be useful here, want to further investigate pyre and/or alternatives, so keeping on near-term backlog.

ghost commented 2 years ago

Picking up on this a little bit:

I'd like to try to implement pysa (pyre's name for the taint analysis tool; it's the same thing that's linked in the original post) as a part of the CI. The idea would be to apply it to PR's, diffing the output of the taint analysis on the PR and the branch it'd be merging into.

Of particular note: pysa doesn't specifically target vulnerabilities, but simply does taint analysis, which can in turn lead to identifying vulnerabilities. This means that it can be quite verbose, so the aim here is to start with a config that won't overwhelm developers with tons of warnings. Further work can allow us to minimize the noise and get more relevant output.

I'm aiming to add a draft PR soon

cfm commented 2 weeks ago

Based on https://github.com/freedomofpress/securedrop/issues/6111#issuecomment-1012033241, I think it might be worth a research spike someday soon on Semgrep's cross-file taint analysis, in service of the requirement that "attacker-provided text should be rendered as plaintext".