We often do code review for companies. Code review is a great control whether or not you are running SAST or other tools, and we look for different things depending on the circumstances. Sometimes the review identifies gaps in authorization that tools can’t find anyway. Other times the language (eg. Clojure or Elixir) is poorly covered by tools, and clients want assurances that the code is being written securely.
In either case, some of what we are doing requires us to understand specifically what the code is doing. But some parts are just looking for known bad things that we often find.
We have used grep. We built a proprietary tool using Clojure back in 2013-2014. Recently we thought this might be a useful thing to revisit and rethink in Golang. This post outlines our thinking and points to the initial release of the open source tool that we built, CRUSH.
In CRUSH we wanted to rethink the idea of a code review assist tool. We wanted to have a way to control the sensitivity so that we would get what we wanted under some different circumstances.
For example, sometimes we want to review every occurrence of a given pattern because it could easily lead to an issue. Things like the following injection vectors merit review just because they exist.
(read-string # Clojure
clojure.java.shell # Clojure
shell_exec # Ruby
Runtime.exec # Java
ProcessBuilder # Java
System.cmd # Elixir
They can be used safely but is a big enough deal if they don’t that we want to see them every time.
Other times we are doing a deeper review and maybe we want to check for SQL InjectionInjection is a type of cyber attack where an attacker exploits vulnerabilities in an application or system to inject malicious code or commands. Injection attacks are often carried out by inserting code or commands into input fields or parameters that are passed to an application or database.. Well, in that case, we can even just look for queries. That might look like:
Ecto.Adapters.SQL.query!
Ecto.Query.API.fragmentEcto.Query.API.fragment
Of course, then we need to trace the path of variables coming into the query to ensure that they are safe and applied correctly. But at least we have a comprehensive list and starting point.
To summarize, we wanted a tool that would:
That’s the start of what Crush was. From there, we externalized the whole idea of a check. Soon we’ll be able to supply a feed to an external set of checks or you can write your own and only use the ones you want.
In order to work with findings consistently across tools, we originally created an OWASP project Open Finding Format. It provides a simple JSON schema for findings.
To make it easier to work with, we are also working on Finding Toolkit - FKIT to be able to create and compare findings easily. FKIT is integrated into CRUSH but may be useful on its own and may be subject for a future blog post.
Sometimes we just want to make sure we’re seeing things. So we might just do something like this:
crush examine --directory /our-code/our-project
--tag injection
--threshold 7.0
--ext clj
This would just look for injections in clj Clojure files. It would only report the issues that have a threshold higher than 7 out of 10. If we get too many results, we can turn the threshold down. We can search for other specific things or more broadly.
This shows how we can compare the current findings to a previous run. First we run the tool.
crush examine --directory /client-code/abcservice --tag clojure > ./testing-clojure.json
… Then we fix things …
Then we run the tool again with the --compare flag. The file referenced is just a JSON array of findings in OFF. The result of the second command will be only the findings that are identified in this run that WERE NOT in the previous run.
crush examine --directory /client-code/abcservice --tag clojure --compare ./testing-clojure.json
We are working on a way to integrate this into GitHub Actions and the PR flow so that this type of code review assist can happen with every pull request.
We definitely want to support custom checks so that you can write your own file to add new checks. We also want to make it so that a set of checks could be a URL so that as the checks evolve, it doesn’t require further work on a CI/CD process to get the latest checks.
We are also working on better documentation for:
We may spend time curating better checks and tuning thresholds.
GitHub Source
Releases
Docker Image
Open Finding Format
FKIT