[e-lang] Bug (0.8.26g): TextWriter/__printOn authority leak

[Kevin Reid]

On Apr 15, 2004, at 23:32, Mark S. Miller wrote:

> * I also when through all the implementations of __printOn written in 
> E,
>  fixing as needed to ensure that they all declare their parameter as
>  ":TextWriter". While doing this, I noticed one place in your den code 
> where
>  you make this same error (in makeFancyRevoker.emaker):
>
>           to __printOn(out) :void {
>              if (Ref.isBroken(underlying)) {
>                out.print(`<revoked: ${Ref.optProblem(underlying)}>`)
>              } else {
>                out.print("<revokable>")
>                out.print(underlying)
>              }
>            }

I noticed this and have fixed it. Unfortunately the CVS server for Den 
is down right now.

> This serves as a good example for why we need to guard __printOn's 
> parameter
>  if we're going to make calls like 'out.print(<internals>)' : If a 
> malicious
>  client passes in a non-TextWriter to this method, they can obtain 
> direct
>  access to the underlying, which is what your forwarder is trying to 
> prevent.
>  (The E source tree had several errors of this form.)

Perhaps, given the ease with which this mistake can be made, and the 
severity of the consequences, the E 
parser/expander/compiler/implementation should give a warning if it 
finds e??`method __printOn(@{x ? (x !~ epatt`@_ :TextWriter`)}) {@_}`?

Or, see below.

>  * Given the above fixes, the revocation logic for printing only needs 
> to be in
>  TextWriter. So, finally, I modified TextWriter with custom revocation 
> logic,
>  essentially as you originally suggested. Besides doing the 
> try/finally as
>  already discussed, I create the sub-TextWriter as follows:
>
>             TextWriter sub = new TextWriter(this,
>                                              myNewline,
>                                              myAutoflush,
>                                              false,
>                                              myContext);
>
> The only important difference is that the parent and child share the 
> same
>  context map. Otherwise, these changes would have broken the 
> cycle-breaking
>  logic.

Oops.

>  Altogether, I feel rather discouraged about borrowing the printOn 
> pattern
>  from Smalltalk into E. Unlike either the Java toString() pattern or 
> the
>  Data-E uncall/uneval serialization pattern, the printOn pattern is
>  surprisingly hard to secure. The "surprisingly" is worse news than the
>  "hard". I'm thinking that I should deprecate __printOn in favor of 
> one of
>  the other two patterns.

toString is unsuitable due to not handling cycles, of course.

Data-E is not fundamentally easier. It only looks so because 
__optUncall is a non-amplifying interface (provides no additional 
authority), whereas __printOn is (allows anything passing the 
TextWriter guard to traverse the object(s-to-be-printed) graph, which 
is often not available to regular clients of the object.

The argument guard of __printOn is equivalent to this, which is very 
much like an __optSealedDispatch-based uncall:

   to __optSealedDispatch(brand) :any {
     switch (brand) {
       match ==printBrand {
         return printSealer.seal([["print", "<revokable>"], ["quote", 
underlying]])
       }
       match _ {}
     }
   }

Hm. I now see that the above interface has the advantage of being 
fail-stop - implementations can't have a security hole by omitting the 
guard.

Unfortunately, it's syntactically ugly.

A way to make __printOn safe: use a transforming guard, such that the 
argument to __printOn is not usable for printing without the 
transformation.

Some untested implementations follow, all of which would work with 
existing __printOn methods (assuming safeScope["TextWriter"] is 
replaced).

(If anything like this is used, I suggest making the sealer and/or 
stamp available in the privileged scope so that custom printers could 
be defined.)

Implementation #1:

   def [textWriterSealer, textWriterUnsealer] := <elib:sealing.Brand>
   interface _TextWriter guards TextWriterStamp {}

   def textWriter implements TextWriterStamp {
     to print(obj) :void {
       obj.__printOn(textWriterSealer.seal(textWriter))
     }
   }

   def TextWriter extends __templateGuard(TextWriter) {
     to coerce(var specimen, optEjector) :any {
       if (textWriterUnsealer.optUnseal(specimen, any) =~ x :notNull) {
         x
       } else {
         return _TextWriter.coerce(specimen, optEjector)
       }
     }
   }

Implementation #2:

   interface _TextWriter guards TextWriterStamp {}

   def textWriter implements TextWriterStamp {
     to print(obj) :void {
       obj.__printOn([textWriter])
     }
   }

   def TextWriter extends __templateGuard(TextWriter) {
     to coerce(var specimen, optEjector) :any {
       if (specimen =~ [x]) {
         specimen := x
       }
       return _TextWriter.coerce(specimen, optEjector)
     }
   }

Implementation #3:

   def [textWriterSealer, textWriterUnsealer] := <elib:sealing.Brand>

   def textWriter {
     to __optSealedDispatch(brand) :any {
       switch (brand) {
         match ==(textWriterSealer.getBrand()) {
           return textWriterSealer.seal(textWriter)
         }
         match _ {}
       }
     }
     to print(obj) :void {
       obj.__printOn(def stub {
         to __optSealedDispatch(brand) :any {
           switch (brand) {
             match ==(textWriterSealer.getBrand()) {
               return textWriterSealer.seal(textWriter)
             }
             match _ {}
           }
         }
       })
     }
   }

   def TextWriter extends __templateGuard(TextWriter) {
     to coerce(var specimen, optEjector) :any {
       if (textWriterUnsealer.amplify(specimen) =~ [x]) {
         x
       } else {
         throw.eject(optEjector, "not a TextWriter")
       }
     }
   }

>  As capability programming patterns grow and adapt conventional object
>  programming patterns, it will be very important to notice and record 
> such
>  surprises. I wouldn't be at all surprised if there were other such 
> nasty
>  surprises already in the E source tree. Sigh.

-- 
Kevin Reid