timll
commented
1 year ago This merge request changes the semantis of taint wrappers. At the moment, we do not models that simply retain taint. If a callee removes a taint, that is explicitly modeled using a
clear flow. I even remember code that drops identity summaries, i.e., summary flows with equalfromandto. Therefore, I'm quite reluctant to merge such a fundamental change without very careful consideration and evaluation.
First, I thought the classSupported boolean would ensure that when we have summaries for the class, but not for the method itself that the taint is kept.
https://github.com/secure-software-engineering/FlowDroid/blob/2296673cf9990e01b76823f7717950bfbca2768c/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/SummaryTaintWrapper.java#L526-L531
Looking at the summary resolving, classSupported is only set when there is a method match. IMO line 58 is dead code here but what should be executed if there is a class summary but no method summary.
https://github.com/secure-software-engineering/FlowDroid/blob/2296673cf9990e01b76823f7717950bfbca2768c/soot-infoflow-summaries/src/soot/jimple/infoflow/methodSummary/taintWrappers/resolvers/SummaryResolver.java#L41-L59
I have pushed some changes that should keep the current semantics of identity on not explicitly mentioned methods in summarized classes while allowing to omit the identity on unrelated classes.
This merge request changes the semantis of taint wrappers. At the moment, we do not models that simply retain taint. If a callee removes a taint, that is explicitly modeled using a
clear flow. I even remember code that drops identity summaries, i.e., summary flows with equalfromandto. Therefore, I'm quite reluctant to merge such a fundamental change without very careful consideration and evaluation.