[e-lang] An attack on a mint

Mark Miller erights at gmail.com
Mon Mar 3 06:55:28 EST 2008


On Sun, Mar 2, 2008 at 5:34 PM, David Wagner <daw at cs.berkeley.edu> wrote:
> I'd like to propose an attack on the Mint described in the
>  Caja spec.  (I couldn't find the definitive, up-to-date description
>  of the MintMaker pattern, so I don't know if this attack applies
>  generally.)

The canonical explanation of the MintMaker pattern is in the online Ode at
http://www.erights.org/elib/capability/ode/ode-capabilities.html#simple-money


> Would anyone be interested to take a look and see whether
>  I've missed anything?
>
>  I've reprinted the Mint code from the Caja spec:
>  [...]
>         deposit: function(amount,src) {
>           var newBal = caja.enforceNat(balance+amount);
>           var box = src.getDecr();
>           brand.unseal(box)(amount);
>           balance = newBal; // See Footnote 1
>         }

>  Footnote 1: The Caja spec actually has "balance += newBal;", but
>  I assume that's a typo.

>  The problem is that Purses are not safe for
>  reentrant calls.
>
>  Does this attack look correct to you?

Yes.

>  Is it already known?

No.

> Does it affect other variants of the MintMaker?

The deposit method of the canonical MintMaker is

                to deposit(amount :int, src) :void {
                    unsealer.unseal(src.getDecr())(amount)
                    balance += amount
                }

An earlier JavaScript version had a += as well; hence the typo.
However, because JavaScript doesn't have integers, I pulled the
addition out into a separate statement to ensure it didn't loose
precision. The enforceNat function is

  /**
   * Enforces that <tt>specimen</tt> is a non-negative integer within
   * the range of exactly representable consecutive integers, in which
   * case <tt>specimen</tt> is returned.
   * <p>
   * "Nat" is short for "Natural number".
   */
  function enforceNat(specimen) {
    enforceType(specimen, 'number');
    if (Math.floor(specimen) !== specimen) {
      fail('Must be integral: ', specimen);
    }
    if (specimen < 0) {
      fail('Must not be negative: ', specimen);
    }
    // Could pre-compute precision limit, but probably not faster
    // enough to be worth it.
    if (Math.floor(specimen-1) !== specimen-1) {
      fail('Beyond precision limit: ', specimen);
    }
    if (Math.floor(specimen-1) >= specimen) {
      fail('Must not be infinite: ', specimen);
    }
    return specimen;
  }


>  In a language with static typing, this particular attack could be
>  prevented by making Purse be a final class and declaring the deposit()
>  function to accept an integer and a Purse.  This prevents creation of
>  fake purses and ensures that we can trust the behavior of src.getDecr().
>  However, it isn't enough to fix the MintMaker.

Dynamic trademarking would have been equally effective here.

In Caja, one might be tempted to use "instanceof" as a trademark
check. But until Caja has the equivalent of final classes -- object
types you can't inherit from -- this would not help.


>  (I seem to recall discussing this second attack when we did the
>  Waterken security review.  I think Tyler may have already applied
>  the second transformation to defeat the second attack -- though I
>  cannot remember.

I don't remember either, and I'm curious. Tyler?

I do remember a different plan interference vulnerability that both
Tyler & I independently fell into in our respective implementations of
the IOU Mint (in Waterken/Joe-E and E respectively). By aborting a
plan with a thrown exception, an attacker could violate conservation
of currency. This bug was fixed well before the Waterken security
review. But it corroborates the suspicion that we may all be
underestimating plan interference vulnerabilities.


> I'll note that the MintMaker in "The Ode" already
> does contain this fix.)

Yes. Since it came, first, I'd prefer to say that the version in the
Caja spec instead contains new breakage.

Even though the Ode got this right and the breakage is new, it's still
disturbing that it was so easy for me to introduce this vulnerability
without noticing. Again, as I review all the tools at disposal of
programmers as well as all the tools our community has in development,
it's not clear that any of these would have helped catch this error.


>  1) I'll notice that, with the second fix, it's effectively as though the
>  first thing the deposit() method does is to call src.getDecr().  Of course
>  that step is something deposit()'s caller could have done itself;
>  deposit() just does it as a convenience to its caller.  When invoking
>  untrusted code is the first thing you do, and if the caller could have
>  done it itself, there doesn't seem to be much of an opportunity for
>  plan interference.  Similarly, if the last thing you do is to invoke
>  untrusted code, there doesn't seem to be much of an opportunity for plan
>  interference (but we must be careful: if you invoke untrusted code, then
>  return a value that you had saved from before, you can easily have plan
>  interference -- so in this case, we'd better say that invoking untrusted
>  code was the next-to-last thing you did, and the last thing you did was
>  to return a value.)  The greatest risk of plan interference comes when
>  invoking trusted code in the middle of a sequence of steps.
>
>  So, this suggests a principle for code reviewers: be most alert for
>  plan interference hazards when invoking untrusted code in the middle of
>  a sequence of steps.  Likewise, a principle for code authors: you can
>  make it easier to review code for plan interference hazards by moving
>  invocations of untrusted code to the very first (or last) thing you do and
>  ensuring that the caller could have done that invocation itself instead.

This also supports the recommendation (which I'll claim is implicit in
my thesis) that, when you can, you should prefer to cross trust
boundaries with asynchronous eventual sends rather than immediate
calls.



>  2) Types can help ensure trustworthy behavior of objects you're relying
>  upon.  Here's a suggested coding pattern: All methods that are part
>  of a security perimeter should check that all untrusted values (their
>  arguments, any values received from untrusted code) are of the expected
>  type before using them.  Also, all relied-upon types should be made final
>  so they cannot be subclassed or extended by untrustworthy code.  Then,
>  when you see a method call on an object whose is known and known to be
>  final, you can rely upon the behavior of that method call.
>
>  This coding pattern is neither necessary nor sufficient for security,
>  but it seems to improve robustness against some kinds of attacks.  It is
>  most natural in a statically typed programming language, but can also
>  be applied in dynamically typed languages with guards (like E).  Also,
>  you can use sealer/unsealer pairs to simulate this pattern, though it
>  becomes more clumsy.

Dynamic trademarking, as in E, is narrower than static types: Ping's
auditor has a trademark per brand. Likewise, were the MintMaker in E
enhanced to do trademarking, it would be most natural to have a
trademark per currency. Trademarking has none of the clumsiness of
using sealer/unsealer pairs for this purpose.

Amusing historical note:
* sealer/unsealer pairs are like an oo model of public key encryption.
* trademarking is like an oo model of public key signatures.
Both come from James Morris' 1973 paper
<http://www.erights.org/history/morris73.pdf> which predated public
key crypto by several years.


>  P.S. I don't know if e-lang is the best place for this email.  Feel
>  free to forward it elsewhere if there is some more appropriate place
>  for this discussion.

e-lang is good. I'll post a pointer on google-caja-discuss.

-- 
Text by me above is hereby placed in the public domain

    Cheers,
    --MarkM


More information about the e-lang mailing list