You want to find vulnerabilities in your code as quickly as possible, which means you need to automate this process. How exactly can you automate vulnerability detection? There's dynamic security testing, and there's static testing—both have their advantages and disadvantages. Today, we'll take a closer look at static testing, using its use at Odnoklassniki.
What principles should you use to choose a static testing tool? What challenges do you face once you've made your choice? How do you write your own analysis rules that extend standard functionality? I've explored all these questions—and now I'm sharing what I've learned with Habr.
This article will focus on Java, web applications, SonarQube, and Find Security Bugs, but it is also applicable to other languages and technologies.
This post is based on my presentation from the Heisenbug conference: if you prefer video, you can watch it; if you prefer text, read on.
Plan:
Preface
An obvious way to automate web application security testing is to use vulnerability scanners, such as OWASP ZAP or Accunetix .
These scanners employ fuzzing , which poses several challenges:
- There is a combinatorial explosion in the number of requests executed during testing: multiply the number of parameters in the application by the size of the list of values for fuzzing and we get a load that not every testing infrastructure can withstand;
- The fuzzer does not remember the state of the application and does not take into account the logic of its operation, and therefore misses some vulnerabilities;
- When adding new forms, pages, and APIs to the application, the scanner needs to be additionally trained.
In a complex system, all this complicates the security testing process, and we turned to static analysis.
What did we need?
- Cheap scaling: an automated tool that can be run frequently, on a large number of applications, and does not require a significant amount of manual effort;
- The ability to use our existing knowledge about previously discovered vulnerabilities (if we have already fixed some bugs, it would be nice to find all similar ones);
- A way to search for vulnerabilities not only on the web, but also in mobile applications.
With such a list of expectations, we tried to implement static analysis.
But before we move on to the story about this, let's look at how static analysis can be used to find vulnerabilities and how this fits into the above-mentioned wishes.
Theory
The idea of static analysis (the application of formal methods to program analysis) is not new. The first static analyzer known to mankind was Lint, released in 1979, making it older than many readers of this text. Lint treated code as text and searched for substrings—essentially no different from grep.
Generally speaking, static analysis can offer two approaches to finding vulnerabilities:
The first is used to search for code constructs that could potentially lead to security issues: calls to insecure APIs, deprecated functions, hard-coded passwords or encryption keys, and so on.
The second approach is dataflow or control flow analysis. If we can build a model that describes the order in which code constructs are executed or how data is propagated, we can make statements about the specifics of this propagation and thus identify vulnerabilities.
To explain to a static analyzer exactly what a vulnerability in a program looks like, we need to write a rule. Static analyzers sometimes call these rules "detectors."
The detector formulates a requirement for the source code (for example, “SQL query strings do not contain externally modifiable data”) and uses models built by the static analyzer to check whether this requirement is met.
What information is available to detectors?
First, there's the abstract syntax tree (AST). The AST's nodes contain metainformation about each program element. It's essentially a parse tree built by the compiler.
A fragment of Java code in the form of an abstract syntax tree would look something like this:
Here are some rules that could be implemented based on the data from the AST:
- "a class that overrides equals() must also override hashCode()"
- "string literals must not be assigned to variables named password or secret".
Secondly, static analyzers construct a control flow graph (CFG). A CFG describes the transitions between basic blocks in a program, allowing one to obtain all possible execution paths.
The control flow graph is constructed based on an abstract syntax tree. Detectors use the CFG to verify assertions about the order of instruction execution. The CFG is also used to construct another important model—the data flow graph, which, in turn, is needed for taint analysis.
There doesn't seem to be an adequate Russian translation of this term, and the easiest way to explain it is with an example. Here's a common cross-site scripting (XSS) vulnerability in its simplest implementation:
String foo = request.getParameter("foo");
response.getWriter().write(foo);
In these two lines, the servlet manages to allow an attacker to execute arbitrary JavaScript in the user's browser.
What's going on here? We see the variable "foo," which contains the value from the request parameter unchanged. The application writes it to the response body. That is, if the foo parameter contains a string containing HTML tags, the user's browser will interpret it as a fragment of HTML markup.
Taint analysis is used to search for such vulnerabilities: various types of injections, cross-site scripting, personal data leaks—anything related to the transfer of parameters in a program.
How does it work? First, the analyzer identifies all points where externally modifiable data could enter the program. These points are called tainted, from the English word "taint" meaning "to corrupt" or "to infect." In our example, the foo parameter is tainted because getParameter returns a value controlled by the attacker.
How does a corresponding detector understand which methods can supply externally modifiable parameters? It contains a list of signatures. This list will obviously be specific to a language, technology, or framework. You can expect a static analyzer to include rules for standard or popular APIs out of the box, but you can supplement them if you have specific knowledge of your code.
So, we've found an entry point that could be the source of the vulnerability. The next step is to find all attempts to use this variable in potentially unsafe contexts. In the example above, the variable's value is used to generate the server response, making the application vulnerable to XSS; using it to generate an SQL query would be a well-known SQL injection vulnerability. Using such a variable in a filename could give an attacker access to the file system, and so on.
To detect potentially unsafe calls, the detector operates in a similar way to tainted: it searches a pre-known list of signatures and designates them as sink, from the English word "to seep" or "to penetrate."
And if there is a path from tainted to sink in the analyzed application, then we can conclude that there is a potential vulnerability.
Let's assume we've fixed the vulnerability and escaped all special HTML characters in the foo parameter, making its further use safe. For this case, the analyzer also has a list of sanitizer signatures—methods that sanitize data. They interrupt the chain between tainted and sink. This means that if we encounter a data-sanitizing method on the path from tainted to sink, there shouldn't be a vulnerability warning.
It would seem that we have everything we need to search for vulnerabilities in our code.
As always, things aren't as simple as they seem. Unfortunately, static analysis has a number of limitations. We'll discuss two of the most annoying ones for web applications below.
The first is the use of reflection. Reflection is actively used in Dependency Injection frameworks and elsewhere, preventing the static analyzer from constructing a control flow graph and, therefore, performing taint analysis.
It's clear why this happens. The static analyzer doesn't recognize that the bar() method is called in the given code fragment. Consequently, if the bar() method is included in some sink signature list, the analyzer won't recognize it as being called.
This problem can be partially solved by adding additional rules that take into account the specifics of a particular project.
Another limitation of static analysis is the generation of source code during program execution.
When it comes to web applications, the most common example is template engines. Unlike JSPs, which are precompiled into Java code and can be parsed, modern template engines are static HTML files with the ability to include evaluated expressions. These expressions are evaluated at runtime, and the results are inserted into the template.
In this case, we cannot trace the call chain, and taint analysis will likely be incomplete.
Choosing a static analyzer
There are many static analyzers for Java, including both open-source projects and commercial products. It's important to define the tool's requirements. Here are our recommendations:
- The first is obvious: support for the technologies we use. At Odnoklassniki, we write in Java, we have JavaScript and TypeScript, and we also want to scan mobile apps, so we need Android support.
- Second, we definitely want taint analysis. As we just discovered, this is needed to find injections and similar bugs, which make up a significant portion of all problems.
- We realized that we wouldn't limit ourselves to a set of standard rules, because our code contains constructs that the static analyzer doesn't understand out of the box, and the ability to customize the rules is important to us.
- We want developers to have a single point of truth about our static analysis: everyone to use the same version of rules, know the current scan status, and see which errors are false positives and which are real bugs. Basically, we need a collaboration server.
- Finally, we want to ensure that adding two lines of code to our project doesn't require us to redo everything. If we've already sorted the scan results into real bugs and false positives, then the next scan, with minor modifications, would allow us to reuse the previous results.
There are publicly available studies comparing various static analyzers. Let's look at one conducted by the OWASP (Open Web Application Security Project) in 2016 ( https://github.com/OWASP/benchmark ). OWASP wrote a benchmark—a Java application with known vulnerabilities—and scanned it with several scanners. The results are below:
The diagram isn't exactly intuitive, but let's take a look at what it depicts. The red dotted line is the result corresponding to a random guess. Anything above this line is better.
The diagram also shows a correlation between the analyzer's recall (how many of all bugs were found) and accuracy (how many of the found bugs were actually bugs). Scanners with fewer false positives missed more bugs.
Taking this into account, we see that all tools produce roughly the same results, with no clear winner. Each tool has a different balance between real vulnerabilities and false positives, but overall, the ratio is roughly the same. There's no clear winner, and commercial scanners don't show significantly different results than open-source ones.
OWASP also tested the same Find Security Bugs scanner with different rule sets, and in this case, it was clear that the more rules, the more vulnerabilities detected. This means that adding rules clearly yields results.
What conclusion can we draw? The choice of engine in a static analyzer is likely less important to us than the power of the ruleset and the ability to customize detectors to suit our needs.
We decided to use the open-source Find Security Bugs . It's a plugin for the well-known static analyzer for Java, formerly known as FindBugs and now called SpotBugs. Find Security Bugs adds a variety of security-related detectors, including for Android.
It is also the only open source analyzer for Java that allows you to add your own taint analysis rules.
Everything related to collaboration, teamwork, and the server side of our tool is handled by SonarQube . Find Security Bugs has a SonarQube plugin that allows you to upload reports to the server and manage scan results.
Practice
Let's try putting this into practice. For example, let's look at a vulnerable web application built on Spring and Thymeleaf.
I specifically chose frameworks that aren't supported by the standard Find Security Bugs ruleset so we could write our own detectors. The example code is available on GitHub .
Example 1
The post already mentioned such a bug: XSS, the use of a value from a request parameter when generating HTML markup.
@ExceptionHandler({IllegalArgumentException.class})
public void oops(HttpServletRequest request, HttpServletResponse response) {
String originalURL = request.getRequestURL() + "?" +
URLDecoder.decode(request.getQueryString());
// ...
PrintWriter writer = response.getWriter();
writer.write("<h1>Error procesing page " + originalURL + "</h1>");
writer.flush();
// …
}
We already know how static analyzers work.
originalURL will be marked as tainted because request.getQueryString() uses the value from the HttpServletRequest without prior validation.
And in writer.write() we have a sink because we write the “corrupted” value into the response body.
We launch the analyzer, and it generates a report that indeed indicates that these lines contain a potential vulnerability:
Example 2
@GetMapping("/photo")
public String photo(@RequestParam("id") long id, Model model) {
Photo photo = photoRepository.findOne(id);
// …
m.addAttribute("photo", photo)
// …
return "/photo";
}
<div th:each="comment : ${photo.comments}">
<p th:utext="${comment.text}"></p>
</div>
Here we have a Spring endpoint that displays a photo with comments to the user. The static analyzer doesn't find any issues. But is everything really OK with this code?
Actually, no. Calling photoRepository.findOne(id) can return data containing user input, meaning it's tainted. These values are then used in the template in an unsafe way (th:utext outputs the string unchanged). If the comment text contains HTML tags, they will break the markup and be interpreted by the HTML browser.
Let's help Find Security Bugs find a vulnerability in this code.
In this case, we can come up with two detector options.
First, we can tell our analyzer that all attempts to output unescaped text in templates (i.e., all mentions of "ph:utext") are potential vulnerabilities. We'll likely find all the vulnerabilities and a few more false positives.
Another way is to look for all attempts to add unverified data to the template engine context.
Let's add a rule that marks addAttribute() as a sink, and then our analyzer will be able to build a call chain from taint to sink and track potential vulnerabilities.
Find Security Bugs allows us to specify lists of these signatures in a config file. We can simply pass it a file with the signatures of these methods. The call to Find Security Bugs and the file contents might look like this:
The bottom line describes the sink—the addAttribute() method. It contains the fully qualified class name, the method name, the argument list, the return type, and the argument number, which is where "corrupted" values cannot be passed.
It's great that Find Security Bugs doesn't force us to write these lines by hand, but provides a calculator that allows us to generate these signatures from Java interfaces.
Example 3
@GetMapping("/photo")
public String photo(@RequestParam("id") long id, Model model) {
Photo photo = photoRepository.findOne(id);
// …
model.addAttribute("photo", photo);
return "/photo";
}
We have a Spring endpoint again that displays a photo to the user. Is there a problem with this piece of code?
Yes! We forgot to check permissions to see if the current user has permission to view the photo. This results in an IDOR (Insecure Direct Object Reference) error.
This is what a fix for this bug would look like:
@GetMapping("/photo")
public String photo(@RequestParam("id") long id, Model model) {
Photo photo = photoRepository.findOne(id);
// …
User currentUser = getCurrentUser = getCurrentUser();
if (!canAccess(currentUser, author)) {
return "/error/302";
}
model.addAttribute("user", author);
model.addAttribute("photo", photo);
return "/photo";
}
Question: Can we teach a static analyzer to detect such vulnerabilities? Insufficient access control seems to be closely tied to the application's business logic, and it's impossible to write a universal detector for this kind of problem. We don't have a single way to explain to the analyzer what access control is and how it's implemented in our system.
But we can check that our code complies with certain pre-defined authorization and access control rules when we already know how they are implemented in that particular application.
In this case, you could say, "Analyzer, please find all Spring endpoints and verify that each one calls the canAccess() method. If it doesn't, then that's a potential vulnerability."
Let's try writing such a rule. The rule for Find Security Bugs is a Java program.
import edu.umd.cs.findbugs.Detector;
public class IdorDetector implements Detector {
@Override
public void vistClassContext(ClassContext classContext) {
}
@Override
public void report() {
}
}
In Find Security Bugs, all detectors must implement the Detector interface. The library contains several basic detectors that can be reused, but we'll try writing a rule from scratch.
So we need to implement two methods: visitClassContext(), which will do the analysis, and report(), which will report the problem if one is found.
How do you find the bug in the example? As mentioned above, let's find all Spring endpoints and verify that each one calls the method implementing access control:
public void visitClassContext(ClassContext classContext) {
List<Method> endpoints = findEndpoints(classContext);
for (Method m : endpoint) {
checkCanAccessCalled(classContext, m);
}
How do you find all Spring endpoints? How does Spring itself do it? Let's find all methods marked with the appropriate annotations.
REQUEST_MAPPING_ANNOTATION_TYPES = Arrays.asList(
"Lorg/springframework/web/bind/annotation/GetMapping;",
"Lorg/springframework/web/bind/annotation/PostMapping;",
// …
private List<Method> findEndpoints(JavaClass javaClass) {
// …
for (Method m : javaClass.getMethods()) {
for (AnnotationEntry ae : m.getAnnotationEntries())) {
if (REQUEST_MAPPING_ANNOTATION_TYPES
.contains(ae.getAnnotationType())) {
endpoints.add(m);
}
}
}
// …
We list all the annotations that Spring uses (there are actually more, I've omitted some for brevity). We get an abstract syntax tree from the analyzer, select all the methods of all classes, and check whether these methods contain the required annotations. If so, we add them to the list for verification.
All that remains is to test each method. How do we do this? Let's think about a control flow graph, which describes the order in which instructions are executed in our code.
private void checkCanAccessCalled(ClassContext classContext, Method m) {
// …
CFG cfg = classContext.getCFG(m);
for (Iterator<Location> i = cfg.locationIterator(); i.hasNext(); ) {
Instruction inst = i.next().getHandle().getInstruction();
if (inst instanceof INVOKESPECIAL) {
if (CAN_ACCESS_METHOD_NAME.equals(invoke.getMethodName(cpg)) &&
className.equals(invoke.getClassName(cpg» {
found = true;
}
}
}
// ...
We obtain a CFG for each endpoint and check each instruction in it. For all method calls, we check whether the name matches the one we're looking for.
// ...
if (!found) {
bugReporter.reportBug(
new BugInstance(this, "IDOR", Priorities.NORMAL_PRIORITY)
.addClass(javaClass)
.addMethod(javaClass, method));
}
// ...
For all endpoints where it was not found, we report a possible vulnerability.
Find Security Bugs allows you to manage bug priorities in your report, divide bugs into categories, and provide a free-form description for each category.
That's it, the detector is ready. If we run it, the report will indeed contain the vulnerable method.
But is our detector perfect?
In addition to real vulnerabilities, it will probably generate a number of false positives.
Let's look at examples of structures that our detector will mistakenly identify as vulnerabilities:
Controller inheritance:
public abstract class BaseController {
protected boolean canAccess() {
return false;
}
}
public class Controller extends BaseController {
@GetMapping("foo")
public String foo() {
//…
canAccess()
//...
}
Nested calls:
public class Controller {
private boolean canAccess() {
//…
}
private boolean canAccessEx() {
canAccess()
//…
}
@GetMapping("foo")
public String foo() {
//…
canAccessEx()
//…
}
}
Branching:
public class Controller {
private boolean canAccess() {
//…
}
private boolean canAccessEx() {
if (x) {
canAccess()
} else {
//…
}
//…
}
@GetMapping("foo")
public String foo() {
//…
canAccessEx()
//…
}
}
I suggest you, Habrauser, as an exercise, independently correct the detector code to take into account the examples given above
.Conclusions
While still requiring dynamic security testing, static analysis allows for faster and more cost-effective detection of common code vulnerabilities than dynamic scanners.
Unfortunately, in the real world, out-of-the-box scanner results don't always meet the expected quality, necessitating the addition of new, application-specific analysis rules.
The open-source Find Security Bugs provides both a set of standard rules and a framework for developing custom ones, making static security testing accessible to all Java projects.
