Tarin Gamberini

A software engineer and a passionate java programmer

Method parameters and TOCTOU

I have always been fascinated by the Design by Contract approach:

Software always had bugs and always will”. Tired of this defeatist attitude? With Design by Contract, invented by Eiffel Software and one of the most widely recognized breakthroughs in the history of software engineering, you can write complex software and not wake up at night fearing that something, somewhere will go wrong. Design by Contract brings science to the construction of correct and robust software.

From this point of view, unfortunately, Java is not Eiffel. Therefore, even if I would have follow this Bertrand Meyer valuable concept, I had to cope with the language itself. I know in the past years some Java frameworks which implement Design by Contract have been proposed, but I have never had the chance to try them seriously. So what I do is simply checking input method parameters.

Summary

Checking for null

Usually I don’t check at all if a method object might be null, because I hope the method code will cause a NullPointerException after few lines far from the method beginning.

I think that «few lines far» from where the problem has originated is a reasonable trade off between a pure fail-fast (fail-early) approach, and a very impractical null check, repeated for each method parameters, which weighs the code readability down:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26

public String getPromotionalCode(Date currentDate, Date birthDate, Order order) {
    if (currentDate == null) {
        throw new NullPointerException(
                "Design by Contract violation: currentDate MUST NOT be null");
    }
    if (birthDate == null) {
        throw new NullPointerException(
                "Design by Contract violation: birthDate MUST NOT be null");
    }
    if (order == null) {
        throw new NullPointerException(
                "Design by Contract violation: order MUST NOT be null");
    }
    if (birthDate.after(currentDate)) {
        throw new IllegalArgumentException(
                "Design by Contract violation: birthDate MUST NOT be after currentDate");
    }
    if (isTimeToOfferPromotion(
            currentDate.getTime(),
            birthDate.getTime())) {
        return computePromotionalCode(
                currentDate.getTime(),
                order);
    }
    return SORRY_ITS_NO_SPECIAL_OFFERS_TIME;
}

You can surely improve readability, in fact Jim Shore suggest to encapsulate checks inside static methods of a class designed to convey the fail-fast meaning, but still you have to write code to perform the checks:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15

public String getPromotionalCode(Date currentDate, Date birthDate, Order order) {
    DesignByContract.assertNotNull(currentDate, "currentDate");
    DesignByContract.assertNotNull(birthDate, "birthDate");
    DesignByContract.assertNotNull(order, "order");
    DesignByContract.assertFalse(birthDate.after(currentDate),
            "birthDate MUST NOT be after currentDate");
    if (isTimeToOfferPromotion(
                currentDate.getTime(),
                birthDate.getTime())) {
        return computePromotionalCode(
                currentDate.getTime(),
                order);
    }
    return SORRY_ITS_NO_SPECIAL_OFFERS_TIME;
}

I prefer, instead, communicate the Design by Contract violation by javadocs, letting NullPointerException happen not to far from where a null check should have been. The only case when I explicitly check for null is when the code let propagate the method input parameter inside other object’s methods, without ever generating a NullPointerException like occurs for the order parameter:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30

/**
 * @param currentDate MUST NOT be null (Design by Contract).
 * @param birthDate   MUST NOT be null (Design by Contract). MUST be less
 *                    then, or equal to, the currentDate (Design by Contract).
 * @param order       MUST NOT be null (Design by Contract).
 *
 * @return the promotional code, or the
 *         {@link #SORRY_ITS_NO_SPECIAL_OFFERS_TIME} message.
 *
 * @throws NullPointerException     if a Design by Contract violation occurs.
 * @throws IllegalArgumentException if a Design by Contract violation occurs.
 */
public String getPromotionalCode(Date currentDate, Date birthDate, Order order) {
    if (order == null) {
        throw new NullPointerException(
                "Design by Contract violation: order MUST NOT be null");
    }
    if (birthDate.after(currentDate)) {
        throw new IllegalArgumentException(
                "Design by Contract violation: birthDate MUST NOT be after currentDate");
    }
    if (isTimeToOfferPromotion(
            currentDate.getTime(),
            birthDate.getTime())) {
        return computePromotionalCode(
                currentDate.getTime(),
                order);
    }
    return SORRY_ITS_NO_SPECIAL_OFFERS_TIME;
}

Checking for mutable objects

The second step I do is to check which method input parameter is not an immutable object. When I found a mutable object I make a defensive copy, because even if the client changed the parameter value my local method defensive copy would remain unchanged:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

/**
 * @param currentDate MUST NOT be null (Design by Contract).
 * @param birthDate   MUST NOT be null (Design by Contract). MUST be less
 *                    then, or equal to, the currentDate (Design by Contract).
 * @param order       MUST NOT be null (Design by Contract).
 *
 * @return the promotional code, or the
 *         {@link #SORRY_ITS_NO_SPECIAL_OFFERS_TIME} message.
 *
 * @throws NullPointerException     if a Design by Contract violation occurs.
 * @throws IllegalArgumentException if a Design by Contract violation occurs.
 */
public String getPromotionalCode(Date currentDate, Date birthDate, Order order) {
    if (order == null) {
        throw new NullPointerException(
                "Design by Contract violation: order MUST NOT be null");
    }
    if (birthDate.after(currentDate)) {
        throw new IllegalArgumentException(
                "Design by Contract violation: birthDate MUST NOT be after currentDate");
    }
    Order orderDefensiveCopy = new Order(order);
    Date currentDateDefensiveCopy = new Date(currentDate.getTime());
    Date birthDateDefensiveCopy = new Date(birthDate.getTime());
    if (isTimeToOfferPromotion(
            currentDateDefensiveCopy.getTime(),
            birthDateDefensiveCopy.getTime())) {
        return computePromotionalCode(
                currentDateDefensiveCopy.getTime(),
                orderDefensiveCopy);
    }
    return SORRY_ITS_NO_SPECIAL_OFFERS_TIME;
}

Checking for TOCTOU

Finally I check for TOCTOU race condition, which stands for «Time Of Check Time Of Use». As explained in Building Secure Software by John Viega, Gary R. McGraw this precaution makes our method more secure when multi-threading comes into play.

In fact it might happen that just after our thread has checked that birthDate isn’t after currentDate, but before it has created the birthDateDefensiveCopy, another thread might change the birthDate value setting it after the currentDate, making totally worthless the initial Design by Contract check; the other thread might also set birthDate to another totally unexpected value making the method isTimeToOfferPromotion return false when it shouldn’t. A similar thing can be said for order. The core concept of TOCTOU is the existence of a temporal vulnerability window during which another running thread can change the used value after the value itself has been checked.

In order to avoid TOCTOU vulnerability window Joshua Bloch, in his book Effective Java, suggest the following two steps approach:

  • the defensive copy must happen before the check
  • the check must be done on the defensive copies
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

/**
 * @param currentDate MUST NOT be null (Design by Contract).
 * @param birthDate   MUST NOT be null (Design by Contract). MUST be less
 *                    then, or equal to, the currentDate (Design by Contract).
 * @param order       MUST NOT be null (Design by Contract).
 *
 * @return the promotional code, or the
 *         {@link #SORRY_ITS_NO_SPECIAL_OFFERS_TIME} message.
 *
 * @throws NullPointerException     if a Design by Contract violation occurs.
 * @throws IllegalArgumentException if a Design by Contract violation occurs.
 */
public String getPromotionalCode(Date currentDate, Date birthDate, Order order) {
    Order orderDefensiveCopy = new Order(order);
    Date currentDateDefensiveCopy = new Date(currentDate.getTime());
    Date birthDateDefensiveCopy = new Date(birthDate.getTime());
    if (orderDefensiveCopy == null) {
        throw new NullPointerException(
                "Design by Contract violation: order MUST NOT be null");
    }
    if (birthDateDefensiveCopy.after(currentDateDefensiveCopy)) {
        throw new IllegalArgumentException
                "Design by Contract violation: birthDate MUST NOT be after currentDate");
    }
    if (isTimeToOfferPromotion(
                currentDateDefensiveCopy.getTime(),
                birthDateDefensiveCopy.getTime())) {
        return computePromotionalCode(
                currentDateDefensiveCopy.getTime(),
                orderDefensiveCopy);
    }
    return SORRY_ITS_NO_SPECIAL_OFFERS_TIME;
}

Accordingly to the (CWE-367 point of the Common Weakness Enumeration List) this approach is a kind of mitigation which doesn’t perform a check before the use; in fact making the defensive copy is a kind of use.

Do you validate method input parameters?

Let me know how you cope with the validation of method input parameters in order to avoid that bugs bubble up very far from where they have been originated. Do you validate method input parameters? Yes? No? Sometimes?

2 Comments to “Method parameters and TOCTOU”

Posted by Enrico #

Great article Tarin!

However, rather than using java.util.Date, I suggest you to use the immutable java.time.LocalDate.

Posted by Tarin Gamberini # Tarin Gamberini

Thanks Enrico,

You are perfectly right! Preferring immutable objects is a great hints.

It was an old Java 6 example, and Java 8 Date Time API has been a very welcome one to the core Java API.

Cheers,
Tarin

Post a comment

A comment is submitted by an ordinary e-mail. Your e-mail address will not be published or broadcast.

This blog is moderated, therefore some comments might not be published. Comments are usually approved by the moderator in one/three days.