DefaultCajaRewriter should not use 'instanceof'

[GoogleCodeExporter]

The current DefaultCajaRewriter uses 'instanceof' to check the class of
nodes it is accepting. This means that any subclasses pass through
unnoticed. Hence anyone can write subclasses upstream of the rewriter and
tunnel through stuff that renders to any strange weirdness.

DefaultCajaRewriter and all similar TCB code should be checking for exact
class matches on its inputs, not 'instanceof'.

Original issue reported on code.google.com by ihab.a...@gmail.com on 29 Jul 2008 at 12:24

[GoogleCodeExporter]

"DefaultCajaRewriter and all similar TCB code should be checking for exact
class matches on its inputs, not 'instanceof'."

More precisely, exact prototype matches. In other words, use 
.isPrototypeOf(obj).

Original comment by davidsar...@googlemail.com on 29 Jul 2008 at 8:26

• Added labels: Security

[GoogleCodeExporter]

DefaultCajaRewriter is still in Java, not JavaScript, so the original 
description was
correct.

Original comment by erights on 30 Jul 2008 at 2:08

[GoogleCodeExporter]

I think this is effectively fixed since all the relevant classes are final.  
eg, Block is final, FunctionDeclaration is final, etc.  Expression isn't final, 
and the places that use instanceof Expression really mean to allow subclasses 
of Expression.  etc.

Original comment by felix8a on 30 Mar 2011 at 12:40

• Changed state: Fixed

[GoogleCodeExporter]

We were thinking of subclassing Block at some point to introduce a Program 
production and define block as any series of statements that are executed in 
sequence.

Original comment by mikesamuel@gmail.com on 1 Apr 2011 at 1:21

[GoogleCodeExporter]

hm, ok.

this doesn't strike me as a likely security risk, it seems to be mainly about 
how easy it is to reason about the TCB.

there's no way for a maliciously subclassed Node to enter the parse tree, it 
has to be deliberate action by someone integrating with Caja.  I think anyone 
who subclasses Node is committing to becoming a part of the TCB, so we could 
just say that this is outside the scope of what Caja promises.  (like there are 
no promises if someone decides to use their own buggy JVM.)

on the other hand, it's easy to avoid instanceof in most cases, so maybe this 
is worth doing.

Original comment by felix8a on 1 Apr 2011 at 5:50

• Changed state: New

[GoogleCodeExporter]

Original comment by ihab.a...@gmail.com on 30 Apr 2013 at 9:05

• Changed state: WontFix