secure-software-engineering / FlowDroid

FlowDroid Static Data Flow Tracker
GNU Lesser General Public License v2.1
1.02k stars 292 forks source link

Manifest parsing error in AbstractBinaryAndroidComponent #678

Closed firmianay closed 6 months ago

firmianay commented 6 months ago

There is a problem with the parsing of Manifest file. The judgment made here is a bit simple and does not consider that the parameters may come from externally defined strings. In this example, the number 2131034116 corresponding to "switch_global_debug" is used to compare. The result is true, but it is actually incorrect.

// Manifest
android:exported="@bool/switch_global_debug"

// res/values/bools.xml
<resources>
    <bool name="switch_global_debug">false</bool>
</resources>

// R
  public static final class bool {
      public static final int switch_global_debug = 2131034116;
  }
    protected AbstractBinaryAndroidComponent(AXmlNode node, BaseProcessManifest<?, ?, ?, ?> manifest) {
        this.node = node;
        this.manifest = manifest;

        AXmlAttribute<?> attrEnabled = node.getAttribute("enabled");
        enabled = attrEnabled == null || !attrEnabled.getValue().equals(Boolean.FALSE);

        loadIntentFilters();

        AXmlAttribute<?> attrExported = node.getAttribute("exported");
        if (attrExported == null)
            exported = getExportedDefault();
        else
            exported = !attrExported.getValue().equals(Boolean.FALSE);
    }
StevenArzt commented 6 months ago

You're right, that needs to be fixed. Do you have an app with which to reproduce the issue?

INTERNAL for FlowDroid team: We already have a parser for the Android resources file (ARSCFileParser). When analyzing the component, we just need to detect that we have a reference and then look up the ID in the ARSC parser. That should be fairly simple.

firmianay commented 6 months ago

Thanks for reply, you can download the test apk from here: https://count.liqucn.com/d.php?id=70985495906&urlos=android&from_type=web

StevenArzt commented 6 months ago

Fixed in fea8906