Groovy SecureASTCustomizer is harmful

I was looking at Groovy DSL slides from Guillaume Laforge when I noticed about SecureASTCustomizer, which led me to what appers to be the original introduction post from Cedric.

Being able to lock Groovy execution down would enable me to use Groovy in more places, so I did a bit of experiment. But I regrettably have to conclude that this feature is practically unusable. In fact I’d argue that it is actively harmful, as it gives a programmer a false comfort.

The fundamental problem is that Groovy is a dynamic language, yet SecureASTCustomizer works by looking at Groovy AST statically. So it’s very easy for Maloney, a malicious attacker, to bypass many of the checks. For example, Cedric’s post talks about how it can let you blacklist/whitelist classes that can be imported. Well, the actual goal of the programmer is to prevent the class from getting used, and not to get them imported. And sure enough, even if I white list the importable classes to java.lang.Math, Maloney can still do Math.class.forName(‘some.secret.class’) to get a reference to a Class, and therefore render the import restrictions pointless.

Then I thought about disabling access to the getClass() method. But this doesn’t work well either because Groovy allows 5.”class” and 5[“class”] to access properties. To statically prevent this, you’d have to prohibit the array access and a string literal, but that doesn’t leave much of a language!

Many other checks offered by SecureASTCustomizer are equally useless. For example, there’s receiversClassesWhiteList that’s supposed to let you restrict the methods the script can invoke by whitelisting the declaring class of the method. But once again, this is a static check! Groovy compiler doesn’t work very hard to infer types, so much so that it can’t even guess that x==”foo” is a boolean type. Therefore, if you actually try using receiver whitelisting, pretty quickly you’ll discover that you either have to allow Object as a receiver (because Groovy assigns this to every expression when it couldn’t infer the type), which will basically renders the point of whitelisting moot as you can now invoke any method by simply casting the expression to Object.

If you go the other route and disallow Object as a receiver. That will reject almost all non-trivial scripts. Or I suppose you can prohibit a method call, but that doesn’t leave much of a language, does it.

Like I said, I think this is fundamentally a futile approach. You just can’t perform any meaningful static sandboxing on a dynamic language.

Instead, what I think is more fruitful is a dynamic checking. For example, what if the compile-time AST transformation intercepts every method call and property access? That is, transform z=x.y as z=checkedGet(x,”y”), transform x.y=5 into checkedSet(x,”y”,5), and finally transform o.foo(a,b,c) into checkedCall(o,”foo”,[a,b,c]). This does make execution a whole lot slower, but I can now perform meaningful checks. And unlike Java SecurityManager, this is a lot more friendly to libraries and web applications, who cannot take over the entire JVM.

I haven’t actually put together such an AST transformer, but this doesn’t look too hard.

What do people think?

comments powered by Disqus