vade retro Pascal - redundant parentheses revisited
werner at almesberger.net
Tue Oct 22 12:01:10 EDT 2013
[ Came across a security-related article that made me think of an
old and constant gripe of mine. Here's my rant, somewhat off-topic. ]
I've been on a crusade against redundant parentheses for a long time.
They often make it hard to read other people's work and obscure the
A very common use of redundant parentheses in C occurs in Boolean
if ((a > 10) || (a == 0))
The origin of such constructs my be Pascal. In Pascal, the Boolean
operators "and" and "or" have higher precedence than relational
operators. Therefore, in Pascal, all the parentheses in
if ((a > 10) or (a = 0))
are required. Note that Pascal uses "=", not "==", to test for
C gives lower precedence to && and || than to relational operators
and we therefore don't need parentheses in this case:
if (a > 10 || a == 0)
Clean and simple.
Programmers coming from a Pascal background may have brought these
parentheses to C, corrupted those improperly schooled in C operator
precedence, and thus caused this now widespread disease. If you
think of it, it's a bit like how the Black Death came to Europe.
So far, these things are widely known. Now, there's a twist:
Once upon a time, a malicious "bug" was introduced in the Linux
kernel that made use of this sloppy practice to hide its nefarious
purpose. While I didn't make the connection to redundant parentheses
back then, I recently read an article  that rehashed the topic,
and spotted the link this time. The code in question looked like
if ((options == (__WCLONE|__WALL)) && (current->uid = 0))
This is structurally equivalent to
if ((a > 10) || (a = 0))
Note that the second expression is an assignment, not a comparison.
In the example above, the intent is to give a process root privileges
by setting current->uid to zero if there is a certain "magic" pattern
in the "options" argument.
"==" vs. "=" is a common typo in C and can produce nasty and hard to
find bugs. It is so common that modern C compilers warn when seeing
if (a = 0)
warning: suggest parentheses around assignment used as truth value
LLVM (clang) gives the following lecture:
warning: using the result of an assignment as a condition without parenthese
note: place parentheses around the assignment to silence this warning
note: use '==' to turn this assignment into an equality comparison
As LLVM explains, the warning can be silenced by adding parentheses.
This works with both compilers:
if ((a = 0))
Looks ugly and should serve as a hint that it's better to avoid this
sort of assignments entirely. E.g., instead of
if ((a = EXPRESSION))
a = EXPRESSION;
Now, the double parentheses are easy to spot. But, given the
widespread bad practice of using redundant parentheses, it's much
harder to catch the assignment in the malicious case:
if ((a > 10) || (a == 0)) /* ugly but common */
if ((a > 10) || (a = 0)) /* evil */
This could of course also happen as the result of a normal typo. So
by using redundant parentheses, developers not only make their code
harder to read, they also defeat a mechanism that is there to protect
them from likely mistakes.
Let's see what would happen if using only the parentheses C requires:
if (a > 10 || a = 0)
error: lvalue required as left operand of assignment
error: expression is not assignable
Both are right - the expression now becomes the equivalent of
if ((a > 10 || a) = 0)
and the left-hand side of the assignment makes no sense. That's also
why the malicious code really needed the parentheses: not only to
avoid a warning that may draw undesired attention, but to get the
code to compile at all.
But if the expressions were swapped, the code would be syntactically
if (a = 10 || a > 10)
And now, the compiler's warning is the last line of defense.
More information about the discussion