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 Injection. Well, in that case, we can even just look for queries. That might look like:
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:
- Only apply patterns to the appropriate file types
- Had a concept of threshold or sensitivity that would allow us to show more or less noise
- Had tags to be able to run certain checks against certain code
- Had a way to only show us NEW issues from a previous run
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.
A Quick Note About OFF
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 a subject for a future blog post.
Examples of How We Use CRUSH
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
What Is Next
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:
- Check structure
- Other integrations (eg. FKIT)
We may spend time curating better checks and tuning thresholds.
Want to stay up to date with the lastest from Jemurai?
Sign up for our monthly newsletter!