Do not make assumptions when coding
A 3 minutes story written on Oct 2018 by Adrian B.G.
The devil is in the details. Any assumption you make, can lead to a bug, or worst …
I gathered a few famous examples snippets that were based on “normal assumptions” but caused big issues system wide issues.
I want to raise awareness for defensive programming, and stating that it is not a nice to have, but rather a requirement for good code.
The difference between 2 consecutive timestamps is always positive
You would say that in a code like
start = time.Now()
//do stuff
diff = time.Now() - start
the diff
will always be a positive number. Besides the fact that the internal clock can be misleading, this case also affected CloudFlare systems in a leap year. The fix is to explicitly check if diff > 0
use it.
How and why the leap second affected Cloudflare DNS
Time, like any other 3rd party input should not be trusted.
An iterator will never exceed the max value
If you deviate from the “classical” iterator, you can run into issues:
loop:
//do stuff
if ++i == buffer.length
break loop
In this generated C code, the presumption was made that i
will not be modified in the loop. If you combine other modules that can affect i
, you will end up in the nasty situation where i
is bigger then the length, in the Cloudbleed case it allowed access to other users memory.
Play it safe, always do
i >= max or i <= 0
instead ofi == max or i == 0
.
Another example I have found was to presume the iterator value after the loop:
i = 0
for ; i < n - 1 ; i++
//do stuff
if condition == skip_some_indexs
i++
array[i] = "\n";
The code can cause a buffer overflow exception, because i
is modified multiple times, in a non-deterministic way.
Incident report on memory leak caused by Cloudflare parser bug
A response will be sent only once
This example is more abstract than a simple coding problem, and it was found in WiFi security libraries (krack attack):
CLIENT = A
question -> CLIENT
CLIENT -> send us the answer
if answer==correct
trust CLIENT //A
The code works well, and it is a proof of concept, the 4-way handshake is more complex of course. But … what if this happens:
//X finds out about As secret question and:
X -> send us the answer
if answer == correct
trust CLIENT //X
Some implementations were more defensive and were not affected by this attack, doing something like
if X == CLIENT => answer == correct
Other common issues I encountered while doing the research:
- a complex set of password rules, but forgot to [check for an empty] (https://www.owasp.org/index.php/Empty_String_Password) string
- return private fields by reference from a
Get()
function - do not check for [overflow values] (http://projects.webappsec.org/w/page/13246946/Integer%20Overflows)(MAX_INT)
We have to assume the worst and cover all the cases that at a first glance, can never happen!