facebook / mariana-trench

A security focused static analysis tool for Android and Java applications.
https://mariana-tren.ch/
MIT License
1.1k stars 139 forks source link

Tainted data does not flow through a reflected class. #156

Open adityavardhanpadala opened 8 months ago

adityavardhanpadala commented 8 months ago

Bug

Bug description Tainted data does not flow through a reflected class when reflectedmethod.invoke(Object, Object) is executed.

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

        try {
            TelephonyManager telephonyManager = (TelephonyManager) getSystemService(Context.TELEPHONY_SERVICE);
            String imei = telephonyManager.getDeviceId(); //source

            Class c = Class.forName("de.ecspride.ReflectiveClass");
            Object o = c.newInstance();
            Method m = c.getMethod("setIme" + "i", String.class);
            m.invoke(o, imei);

            Method m2 = c.getMethod("getImei");
            String s = (String) m2.invoke(o);

            SmsManager sms = SmsManager.getDefault();
            sms.sendTextMessage("+49 1234", null, s, null, null);   //sink, leak
        } catch (InstantiationException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (IllegalAccessException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (ClassNotFoundException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (NoSuchMethodException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (IllegalArgumentException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (InvocationTargetException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }   
    }

Reproduction steps The apk is from DroidBench https://github.com/secure-software-engineering/DroidBench/blob/master/apk/Reflection/Reflection3.apk

Sink Model:

    {
      "find": "methods",
      "where": [
        {
          "constraint": "signature_match",
          "parent": "Landroid/telephony/SmsManager;",
          "name": "sendTextMessage"
        }
      ],
      "model": {
        "for_all_parameters": [
          {
            "variable": "idx",
            "where": [],
            "sinks": [
              {
                "kind": "TaintedSmsSink",
                "port": "Argument(idx)"
              }
            ]
          }
        ]
      },
      "verbosity": 2
    },

Source Model:

{
  "model_generators": [
    {
      "find": "methods",
      "where": [
        {
          "constraint": "signature_match",
          "parent": "Landroid/telephony/TelephonyManager;",
          "name": "getDeviceId"
        }
      ],
      "model": {
        "sources": [
          {
            "kind": "TaintedSource",
            "port": "Return"
          }
        ]
      },
      "verbosity": 2
    }
  ]
}

Logs Log file with the logging for onCreate method. log.txt

arthaud commented 8 months ago

Mariana Trench won't resolve calls using reflection, since the name can be anything at runtime. In this specific example, we could detect that the argument is always "setImei" and call the right method. We don't currently support that, since we haven't seen this used frequently. If you want a work around, you could model Method.invoke to have a propagation from Arg(2) -> Arg(1), so imei taints o.