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

False negative with taint propagated via class attribute without static initializer #176

Open
draftyfrog opened this issue Jan 29, 2025 · 1 comment

Comments

@draftyfrog
Copy link

Bug

Bug description
This one is very similar to #174 but without static initializer.

Please consider the following code

public class MainActivity extends AppCompatActivity{
  public void onCreate(Bundle savedInstanceState){

    MyClass myInstance = new MyClass();
    myInstance.myField = this.source();

    String myString = myInstance.myField;
    myInstance.myField = ""; // If this statement is removed, Mariana Trench reports the sink in the next line
    this.sink(myString);  // NOT reported as issue by Mariana Trench
}
  public String source(){ // Defined as source in MT config
    return "Secret";
  }

  public void sink(String param){} // Defined as sink in MT config
}

class MyClass{
  String myField;
}

As annotated in the code, Mariana Trench doesn't detect any issues, but actually the sink in MainActivity.onCreate should be reported.

I'm using mariana-trench Version: 1.0.6.

@arthaud
Copy link
Contributor

arthaud commented Jan 31, 2025

Hi @draftyfrog,

This is most likely a problem with our alias analysis. This might actually be fixed, but the version of Mariana Trench you are using is one year old. I will look into pushing a new version when I get the time.

In the meantime, you could try building mariana trench from source and rerunning the analysis. That might solve the false negative.

facebook-github-bot pushed a commit that referenced this issue Feb 3, 2025
Summary:
#176

Aliasing should ideally address this but doesn't.

This is related to the choice of not dereferencing on iget. Solution pending.

Note that aliasing does not regress the behavior, i.e. without aliasing, the false negative would still be there.

Reviewed By: arthaud

Differential Revision: D68990317

fbshipit-source-id: 1ede0349c0ff0481d03d2f8f344d3f38664fff11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants