Security Hotspot review - are your doors locked?

by g. ann campbell|

By some quirk of fate or architecture, there are four doors into my moderately-sized ranch house. That's four distinct points where an attacker or thief could stroll into my home and wreak havoc. If I know every door is locked, I can sleep soundly. But are they? All of them? I can't be sure until I've checked. So every night before bed, I go to each door and check its lock.

Security Hotspots in your code are like the doors in my house; they're places where it might be possible for an attacker to enter. And they need to be double-checked by a human - fortunately not every night! - to make sure they're not leaving the door open to attack. In this post I'll help you understand: what it means in practical terms to have Security Hotspots in your code, why you need to review them, and what tools SonarQube gives you to do that.

Specifically, each Security Hotspot is a security-sensitive piece of code. I.e. it's a place where things could go wrong. Your particular use of that code might be a problem, or it might not. You won't know until you look.

For instance, SonarQube raises Security Hotspots on dynamically executed code.

Dynamic execution is available in most interpreted languages. It allows you to decide at runtime in a very flexible manner exactly what commands to execute. If those commands are supplied or even influenced by an attacker, you've got a problem. But if there's no way an attacker could touch them, then you're fine. It might be a problem. It might not. You won't know until you look. 

That's why SonarQube doesn't raise Security Hotspots as normal issues. Normal issues are definitely problems (barring the odd false-positive) but Security Hotspots need a human to decide. So starting in version 8.2, SonarQube keeps Security Hotspots entirely separate from issues, in a sophisticated new interface.

That interface is designed to help you understand what kind of risk each individual Security Hotspot represents. Once you've got the background to understand the risk potential, you're guided in assessing whether or not there's actually a problem in your situation. 

In most cases, you'll be fine; you can simply mark the Security Hotspot "Safe" and move on. But if there is a problem, you'll find detailed guidance on how to fix it.

Or you can assign the Security Hotspot to a colleague for correction or additional analysis, and move on with your review. 

As you progress through the open Security Hotspots in your project you'll see the project's percentage of reviewed Security Hotspots change in real time. That way you can always tell how much farther you have to go and when you're nearly done. Those changes are also reflected on the project homepage, where you see not just the review percentage, but also the review rating and count of still-open Security Hotspots.

You might be wondering about the difference between a Security Hotspot and a Security Vulnerability. At SonarSource, we created the Security Hotspot concept as a way to distinguish "this requires review" (Security Hotspot) from "this requires a fix" (Security Vulnerability), and that's the essential difference between the two. With Security Hotspots, there might be a problem, there might not. But when we raise a Security Vulnerability we're certain there's a problem, no ifs, ands, or buts about it. For example, here's a Security Vulnerability found with taint analysis:

In this case, there's no question that user-supplied data is sent to the database without being sanitized. I.e. a real problem. But there are plenty of Security Vulnerabilities you can find without taint analysis. For instance, there's this use of a weak protocol:

Even without human validation, there's no question that a stronger protocol should be used; there's a vulnerability and it needs to be fixed. Security Hotspots don't necessarily require a code change; all you know for sure is that they need to be reviewed. 

SonarSource developed the Security Hotspot concept from the ground up as part of our effort to deliver on an essential promise: empower developers to own the security of their code. In addition to writing accurate and valuable rules, that also means making a clear distinction between "must fix" and "must review". Other tools don't do that; they raise Security Hotspots and Vulnerabilities together with the same "drop everything!" priority. That's a shame, because when developers triage a Security Hotspot that was raised as a Vulnerability, they get the sense that the false positive rate is high. As a result, they can lose confidence in the tool and perhaps in SAST (static analysis security testing) as a whole. That's why we feel it's important to make an explicit distinction. There's only so far you'll ever be able to go with static analysis. No analyzer can ever tell whether the data you're storing insecurely in a cookie is sensitive or completely inconsequential. Human wisdom is required in that case, and that's what Security Hotspot review brings to table: clarity, guidance and an opportunity to sit down with your team and discuss secure coding practices.