In this blog post we take a look at a common Java coding mistake (using a bitwise operator instead of a conditional operator), the error it makes our code vulnerable to, and how we can use Sensei to fix and detect the issue.
> "Java Gotcha" - a common mistake pattern that is easy to accidentally implement.
A fairly simple Java Gotcha to accidentally fall into is: using a Bitwise operator instead of a Boolean Comparison operator.
e.g. a simple mistype can result in writing "&" when you really meant to write "&&".
A common heuristic we learn when reviewing code is:
> "&" or "|" when used in a conditional statement is probably not intended.
In this blog post, we will explore the heuristic and identify ways we can identify and fix this coding issue.
Using Bitwise operators with Booleans is perfectly valid, so Java will not report a syntax error.
If I construct a JUnit Test to explore a truth table for both Bitwise OR (|) and Bitwise AND (&) then we will see that the outputs from the Bitwise operator match the truth table. Given this, we might think that the use of Bitwise operators is not an issue.
The test passes, this is perfectly valid Java.
This test also passes, why do we prefer "&&'and "||'?
The real issue is the difference in behaviour between Bitwise (&, |) and Boolean (&&, ||) operators.
A Boolean operator is a short circuit operator and only evaluates as much as it needs to.
In the above code, both boolean conditions will evaluate, because the Bitwise operator has been used:
This leaves my code open to a NullPointerException if args is null because we will always perform the check for args.length, even when args is null because both boolean conditions have to be evaluated.
When an && is used e.g.
As soon as we know that args != null evaluates to false the condition expression evaluation stops.
We don't need to evaluate the right-hand side.
Whatever the outcome of the right-hand side condition, the final value of the Boolean expression will be false.
This is a pretty easy mistake to make and is not always picked up by Static Analysis tools.
I used the following Google Dork to see if I could find any public examples of this pattern:
This is the type of code that might pass a code review because we often do use Bitwise operators in assignment statements to mask values. But in this instance, the outcome is the same as the if statement example above. If intent is ever null, then a NullPointerException will be thrown.
Very often we get away with this construct because we often code defensively and write redundant code. The check for != null may well be redundant in most use cases.
This is an error made by programmers in production code.
I don't know how current the results for the search are, but when I ran the search there were results back with code from: Google, Amazon, Apache... and me.
A recent pull request on one of my open source projects was to address exactly this error.
When I checked my sample code in a few static analysers, none of them picked up this hidden self-destruct code.
As a team at Secure Code Warrior, we created and reviewed a fairly simple Sensei recipe that could pick this up.
Because Bitwise operators are perfectly valid, and often used in assignments we focussed on the use-case of if statements, and the use of Bitwise &, to find the problematic code.
This uses a regular expression to match " & " when it is used as a condition expression. e.g. in an if statement.
To fix it, we again relied on regular expressions. This time using the sed function in the QuickFix to globally replace the & in the expression with &&.
This covers the most common misuse of a Bitwise operator, i.e. when a Boolean operator was actually intended.
There are other situations where this could crop up e.g. the assignment example, but when writing recipes we have to attempt to avoid false-positive identification, otherwise recipes will be ignored or turned off. We build recipes to match the most common occurrences. As Sensei evolves, we may well add additional specificity into the search functionality to cover more matching conditions.
In its current form, this recipe would identify many of the live use-cases, and most importantly, the one that was reported in my project.
NOTE: A fair few code warriors contributed to this example and recipe review - Charlie Eriksen, Matthieu Calie, Robin Claerhaut, Brysen Ackx, Nathan Desmet, Downey Robersscheuten. Thanks for your help.
You can install Sensei from within IntelliJ using "Preferences \ Plugins" (Mac) or "Settings \ Plugins" (Windows) then just search for "sensei secure code"
We have a lot of source code and recipes for these blog posts (including this one) in the `sensei-blog-examples` repository in the Secure Code Warrior GitHub account.