SAP / project-fontus

Dynamic tainting framework for Java applications leveraging on-the-fly bytecode rewriting.
Apache License 2.0
6 stars 2 forks source link

Semantic differences in Taint-Aware Properties class compared to original #29

Open leeN opened 2 weeks ago

leeN commented 2 weeks ago

The Properties class is some weird custom code/inherited HashMap amalgamation. It mostly behaves the same, but during debugging crashing dacapo benchmarks, I found the following incompatibility:

In the eclipse benchmark we have a class that inherits from Properties and overrides the put method, which is inherited from Hashmap. It then fills the Properties object with the built-in loading (i.e., calling load()) functionality.

class MyProperties extends Properties {

    public synchronized Object put(Object var1, Object var2) {
        System.out.printf("Putting: %s -> %s\n", var1, var2);
        return null;
    }
}

class Main {
    public static void main(String[] args) throws Exception {
        FileInputStream fis = new FileInputStream("./msgs.properties");
        MyProperties props = new MyProperties();
        props.load(fis);
    }
}

Without tainting, this will print all key-value pairs from msgs.properties. With tainting, it does not print anything. This is because we are wrapping a Properties object and delegating the load call to said object. Consequently, the overridden put method is not called during loading, as the overridden put method is for the wrapper class.

Generally, this is quite nasty code, but the Eclipse people think it's reasonable, so we have to support it, I suppose.

leeN commented 2 weeks ago

Our options are to override all Properties methods or derive from Properties directly. The latter option seems suboptimal, as the Properties object is somewhat insane :/