Re: An attack on a mint

On 3/3/08, Tyler Close <[email protected]> wrote:
> During the Waterken security review, I think we also determined this
> money destroying bug could be fixed by simply switching the order of
> the two lines:
>
>    balance += r;
>    src.balance = 0;
>
> And as MarkM points out, adding a comment explaining why the order is important.

Since there's some measure of social engineering to expressing this
code such that it will survive maintenance, I'd like to try out a
version to see how people react:

Here's the original method:

    int take(final PurseX src) {
           if (dead) { throw new NullPointerException(); }
           if (src.dead) { throw new NullPointerException(); }
           final int r = src.balance;
           balance += r;
           src.balance = 0;
           return r;
       }

Suppose a rewrite like:

    int take(final PurseX src) {
           if (dead) { throw new NullPointerException(); }
           if (src.dead) { throw new NullPointerException(); }

           // subtract the credits from the source
           final int r = src.balance;
           src.balance = 0;

           // add the credits to the destination
           balance += r;

           return r;
       }

Is such a layout sufficient to discourage reversion to the previous state?

--Tyler