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

Question about writing advanced rules #60

Closed
chuayupeng opened this issue Oct 25, 2021 · 8 comments
Closed

Question about writing advanced rules #60

chuayupeng opened this issue Oct 25, 2021 · 8 comments

Comments

@chuayupeng
Copy link

For reference, I am trying my hand at writing a rule to detect PendingIntents used dangerously, as noted in https://www.researchgate.net/publication/325818237_PIAnalyzer_A_Precise_Approach_for_PendingIntent_Vulnerability_Analysis, and I have a rule that can detect implicit intents being initialised as its source, and something like this for its sink.

{
  "model_generators": [
    {
      "find": "methods",
      "where": [
        {
          "constraint": "parent",
          "inner": {
            "constraint": "name",
            "pattern": "Landroid/app/PendingIntent;"
          }
        },
        {
          "constraint": "any_of",
          "inners": [
            {
              "constraint": "name",
              "pattern": "getB.*"
            },
            {
              "constraint": "name",
              "pattern": "getA.*"
            },
            {
              "constraint": "name",
              "pattern": "getS.*"
            }
          ]
        }
      ],
      "model": {
        "for_all_parameters": [
          {
            "variable": "x",
            "sinks": [
              {
                "kind": "PendingIntentSink",
                "port": "Argument(x)"
              }
              ]
          }]
        }
    }
  ]
}

However, I am unable to abstract this flow as its own standalone source/sink. Would like to develop this further to detect instances of PendingIntents being initialised with implicit intents, and then sent off as another Intent's extraData. How should I link these 2 use cases up? Appreciate any advice regarding this!

@chuayupeng chuayupeng changed the title [Other] Question about writing advanced rules Question about writing advanced rules Oct 28, 2021
@chuayupeng
Copy link
Author

Trying to figure this out still, but was wondering if partial_labels or some other function can be used to chain different types of declarations up?

@arthaud
Copy link
Contributor

arthaud commented Nov 1, 2021

Could you describe what flow you are trying to catch? I don't have access to this paper.
Please give a concrete example.

@chuayupeng
Copy link
Author

Hi @arthaud, sure! The example vulnerable code is here:

protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main_vuln);
    Intent baseIntent = new Intent();
    PendingIntent pendingIntent = PendingIntent.getActivity(this, 1, baseIntent, PendingIntent.FLAG_UPDATE_CURRENT);
    Intent implicitWrappingIntent = new Intent(Intent.ACTION_SEND);
    implicitWrappingIntent.putExtra("vulnPI", pendingIntent); 
    sendBroadcast(implicitWrappingIntent);
}

Basically, I want to find any invocations of PendingIntent, via either getActivity(), getActivities(), getBroadcast()
or getService(), check that the the baseIntent that is initialised within PendingIntent is implicit, and that the PendingIntent itself is wrapped in another implicitWrappingIntent, before being sent out.

So far, I could get the source of the rule to be checking for initialization of a implicit intent, and I can trace up till the getActivity function, but not sure how to chain this rule with a rule that checks that the pendingIntent is then wrapped within another implicit intent.

@gitWK86
Copy link

gitWK86 commented Nov 2, 2021

Like below

protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main_vuln);

    // 1.implicitly IntentCreation Source 1
    Intent baseIntent = new Intent(); 
    
    // 2. implicitly intent source flows into PendingIntent.getActivity, then pendingIntent is tainted
    PendingIntent pendingIntent = PendingIntent.getActivity(this, 1, baseIntent, PendingIntent.FLAG_UPDATE_CURRENT);
    
    Intent implicitWrappingIntent = new Intent(Intent.ACTION_SEND);
    
    // 3. tainted pendingIntent flows into implicitWrappingIntent, then implicitWrappingIntent is tainted
    implicitWrappingIntent.putExtra("vulnPI", pendingIntent); 
    
    // 4. sendBroadcast(0) is sink
    sendBroadcast(implicitWrappingIntent);
}

@chuayupeng
Copy link
Author

How do I ensure that the taint path must have PendingIntent in it though? Or is it not possible to define that sort of granularity within the rules?

@arthaud
Copy link
Contributor

arthaud commented Nov 2, 2021

I don't think there is a good way to model this currently.
I would personally use features to mark that the taint went through PendingIntent.getActivity and Intent.putExtra, but you will probably get a good amount of false positives.
It could be solved with a technic we call Taint Transforms but it is not implemented in Mariana Trench.

@chuayupeng
Copy link
Author

Thanks @arthaud! Will keep that in mind, and work with features for now.

@serrapa
Copy link

serrapa commented Sep 2, 2022

Hello @chuayupeng , could you share how you achieved your goal with features? I am trying to learn propagations and features, which are not well-documented (few examples and not much detailed) and simple to understand, in my opinion.

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

4 participants