Clarity Is Key: How Code Quality Affects Security

Introduction

Secure code reviews are an indispensable practice if you’re serious about building secure software. Software development processes that incorporate secure code reviews are much more effective at producing secure code than those that don’t. Code reviews are necessary because they can detect a wide range of issues that other security measures may not catch; however, any review is only as good as the reviewer. The best secure code experts in the world share the same fatal flaw: They’re human.

One often overlooked aspect of secure software development is the readability of code. It’s commonly accepted that the average human can hold 7 ± 2 items in their short-term memory. This is why phone numbers in the USA are 7 digits (plus an area code). When humans read code, they must build a mental model of the software in their short-term memory. The more complicated the code, the more difficult it is for a human to build that model and, thus, understand the application. Code that is easily readable and comprised of abstractions with clear responsibilities is easier to fit in your head.¹ In short, readable code reduces the likelihood of errors on the part of the author and the security reviewer because readable code enables humans to reach a fuller understanding of the software.

An example using EggTimer

Imagine you’re writing a security-sensitive function that relies on a timer or timeout functionality. For example, you may be implementing rate limiting for user login or removing expired tokens from your database periodically. We’ve all written code to do something similar that looks a bit like this:

from time import time, sleep

def do_security_sensitive_task()
    max_sleep_time_sec = 1.5

    start_time = time()
    timeout_sec = 42.0

    while time() - start_time < timeout_sec:
        # Do or check some stuff

        time_remaining = timeout_sec - (time() - start_time)
        if time_remaining > max_sleep_time_sec:
            sleep(min(time_remaining, max_sleep_time_sec))
        else:
            sleep(max_sleep_time_sec)

When you first encounter this code, you’ll have quite a few questions. What is the purpose of this loop? Oh, I see! It’s a timeout. Is the order of operations correct in the loop condition? Has time_remaining been calculated correctly? Is the if clause correct? Hint: It’s not. Does this code behave properly if the system clock is updated after start_time is set? Hint: It doesn’t. How many times is this code (and its flaws) duplicated within the application?

Now that you’ve answered all of the above questions, you can finally get down to business and evaluate the security of this function. Oops! Would you look at the time! Maybe you’ll get to it tomorrow.

We can do better. EggTimer can help.

from time import sleep

from eggtimer import EggTimer


def do_security_sensitive_task():
    max_sleep_time_sec = 1.5

    timer = EggTimer()
    timer.set(42.0)

    while not timer.is_expired():
        # Do or check some stuff

        sleep(min(timer.time_remaining_sec, max_sleep_time_sec))

Ah, that’s better! Clear, concise, reusable, and expressive. Which example is easier to inspect? Which takes up less space in your head? Which would you rather review? I, for one, would feel much more confident making assertions about the security of the second example (provided I had previously reviewed EggTimer itself).

Conclusion

Any developer worth their salt wants to produce high-quality code. Often, schedule pressure or other factors coerce developers into accepting some amount of technical debt. Sometimes excuses are made, such as, “We’ll clean it up later” or “This code won’t change much anyway.” This is an incomplete assessment of the risks of accepting poor-quality code. In addition to technical debt, decisions such as these also incur “security debt” because convoluted code can hinder security reviews or render them ineffective.

I can tell you, from experience, that the extra time spent writing readable code is an investment that pays dividends. The return on investment is particularly short if secure code reviews are part of your software development process. After all, you’d prefer your security reviewer to spend their time ensuring the security of the software instead of just trying to wrap their head around it, wouldn’t you?

P.S. If you’re interested in using EggTimer in your project, it’s licensed under GPLv3 and can be installed with pip install -U egg-timer.

1. For a deeper discussion on this topic, I highly recommend “Code That Fits in Your Head” by Mark Seemann.