Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Test][FP] TaintTransform on Args #134

Closed
wants to merge 1 commit into from

Conversation

the-storm
Copy link
Contributor

@the-storm the-storm commented Jul 14, 2023

Summary

Minimal test showing a FP on Transform for the following piece of code

void test_fp {
   Object source = Origin.source();
    transformT1OnArg(source);
    Origin.sink(source);
}

Expected to see only one flow which is Source -> T1 -> Sink
Actual: 2 flows

  • Source -> Sink <-- This flow is FP
  • Source -> T1 -> Sink

Comment on lines +3128 to +3193
"rule" : 1,
"sink_index" : "0",
"sinks" :
[
{
"kinds" :
[
{
"call_info" : "Origin",
"kind" : "Sink",
"origins" :
[
"Lcom/facebook/marianatrench/integrationtests/Origin;.sink:(Ljava/lang/Object;)V"
]
}
],
"origin" :
{
"method" : "Lcom/facebook/marianatrench/integrationtests/Origin;.sink:(Ljava/lang/Object;)V",
"position" :
{
"end" : 21,
"line" : 279,
"path" : "TaintTransforms.java",
"start" : 16
}
}
}
],
"sources" :
[
{
"kinds" :
[
{
"call_info" : "Origin",
"kind" : "Source",
"origins" :
[
"Lcom/facebook/marianatrench/integrationtests/Origin;.source:()Ljava/lang/Object;"
]
}
],
"local_positions" :
[
{
"end" : 26,
"line" : 278,
"start" : 21
}
],
"origin" :
{
"method" : "Lcom/facebook/marianatrench/integrationtests/Origin;.source:()Ljava/lang/Object;",
"position" :
{
"end" : 32,
"line" : 277,
"path" : "TaintTransforms.java",
"start" : 27
}
}
}
]
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arthaud This flow is unexpected

@the-storm the-storm closed this Jul 14, 2023
@arthaud
Copy link
Contributor

arthaud commented Jul 18, 2023

Discussed this offline.
This is the expected outcome and not a false positive, since propagations are applied with "weak" updates by default,
which means the resulting taint after a method call is the original taint joined with the effect of propagations.
In this example, that means we end up with 2 flows: one with the transform and one without the transform.
You could use the mode strong-write-on-propagation on transformT1OnArg to avoid this, but be careful as this could lead to false negatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants