Unknown methods made to modify taint state of their parameters to unknown

[formanek]

Besides the String type, I have also excluded parameters of type java.lang.Object - these are not immutable, but cannot be modified without casting to a concrete type, so I think it is quite good approximation and better solution, then adding many methods to method-summaries.txt.

[h3xstream]

This pull request will resolve #76 (Ref)

[formanek]

@h3xstream Well, I'm not sure about the Object class too, but do you know an example of a method, that modifies its Object parameter? If there is a method of some of the Object children, which modifies the instance passed as parameter, there should be an interface or superclass as a parameter type instead of Object.

Yes, primitive types are ignored. But arrays are ignored too, which is a mistake - I'll fix it in a minute.

The field was originally called IMMUTABLE_OBJECT_TYPES, but since I've added Object, I've renamed it to avoid confusion.

Methods can be removed, if we are going to keep Object excluded and if the mentioned methods do not modify any of their array parameters.

[h3xstream]

@formanek For the Object being down cast and modify. I have two examples.

Manual down casting

public void modify(Object data) {
   if(data instanceof Person) {
      Person p = (Person) data;
      p.setName("aaaa");
   }
}

Implicit casting with the use of Generic

class PersonProcessor extends GenericProcessor<Person> {
    public void modify(Person p) {
        p.setName("aaaa");
    }
}

The actual code/bytecode will be similar to:

class PersonProcessor extends GenericProcessor<Person> {
    public void modify(Object p) {
        ((Person) p).setName("aaaa");
    }
}

[formanek]

@h3xstream The first example is not realistic for me - why wouldn't use more concrete type? I've tried the second example and even though a bridge method with Object parameter is generated, method modify has Person type when called on PersonProcessor from your example. Can you provide a code for GenericProcessor and a call of modify that has Object parameter?

[h3xstream]

@formanek https://gist.github.com/h3xstream/0b02944e0017dcb50f54#file-output-result-L21

Some decompiler such as JD-gui will hide the Object type when they understand that Generics are being used.

[formanek]

@h3xstream Ok, thank you. I was calling the method directly on the subclass, so it had the subclass type parameter.

However, I'm still not very convinced, that Object type should not be excluded. I think, If a parameter in a method of a superclass (or interface) is just Object and an instance of the extending (implementing) class is declared as its supertype (interface), then the parameter is not going to be modified. And even if it is modified, then not with a tainted value. I just cannot imagine a real code, that would do this. Do you know some examples from real API or can you just describe me the situation, where an Object parameter could be modified with a tainted value?

In contrast, when we include the Object type, logging API could often make a safe value unknown, since queries can be logged before executing and Object parameters are used here.

[h3xstream]

Ok I see the point for the logging API. These are pretty common for logging query and such.
I would prefer a white list of those API to be more systematic. We could add them progressively.

I understand your point for quickly covering generic API such as logging, metric, etc. But, I prefer the certainty of the analysis over avoiding to maintain certain known safe methods.
Object can literally be anything. It is hard predict the impact on unknown API that people will develop.

One thing to keep in mind. The bytecode can also come from Groovy, Scala and even JSP. This support come for free because it is all bytecode. On the other hand, it is hard to predict that their respective compilers will not produce type erasure.

[formanek]

Fine, Object type is now included. Methods equals are excluded from the mechanism and I have added Java logging API (not tested).

I had to uncomment two methods in method-summaries.txt. I have deleted those having just String arguments (and unimportant objects). The remaining disabled methods (starting with -) could be probably uncommented to improve analysis for collections and arrays - can you check, that there is no method, which taints some of its parameters?