secure-software-engineering / FlowDroid

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

Unsoundness in CHA caused by no points-to analysis #503

Closed amordahl closed 1 year ago

amordahl commented 1 year ago

I've noticed some unsoundness in CHA as compared to the SPARK-based points-to analyses (e.g., RTA, VTA, SPARK).

Test Case

AnonymousClasses1.apk

Here's the code of the APK:

    LocationListener locationListener = new LocationListener() {
        @Override
        public void onStatusChanged(String provider, int status, Bundle extras) {           
        }

        @Override
        public void onProviderEnabled(String provider) {
        }

        @Override
        public void onProviderDisabled(String provider) {
        }

        @Override
        public void onLocationChanged(Location location) { //source
            Toast.makeText(getApplicationContext(), "aa", Toast.LENGTH_LONG).show();
            latitude = location.getLatitude();
            longitude = location.getLongitude();            
        }
    };

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_annonymous_class1);

        // Acquire a reference to the system Location Manager
        locationManager = (LocationManager)getSystemService(Context.LOCATION_SERVICE);

        // Register the listener with the Location Manager to receive location updates
        locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, 5000, 10, locationListener);        
    }  

As I understand, regardless of the call graph algorithm, the following code (from analyzeMethodForCallbackRegistrations) is what is used to scan for callback registrations:

if (arg instanceof Local) {
    Set<Type> possibleTypes = Scene.v().getPointsToAnalysis().reachingObjects((Local) arg)
            .possibleTypes();
    for (Type possibleType : possibleTypes) {
        RefType baseType;
        if (possibleType instanceof RefType)
            baseType = (RefType) possibleType;
        else if (possibleType instanceof AnySubType)
            baseType = ((AnySubType) possibleType).getBase();
        else {
            logger.warn("Unsupported type detected in callback analysis");
            continue;
        }

        SootClass targetClass = baseType.getSootClass();
        if (!SystemClassHandler.v().isClassInSystemPackage(targetClass.getName()))
            callbackClasses.add(targetClass);
    }

    // If we don't have pointsTo information, we take
    // the type of the local
    if (possibleTypes.isEmpty()) {
        Type argType = ((Local) arg).getType();
        RefType baseType;
        if (argType instanceof RefType)
            baseType = (RefType) argType;
        else if (argType instanceof AnySubType)
            baseType = ((AnySubType) argType).getBase();
        else {
            logger.warn("Unsupported type detected in callback analysis");
            continue;
        }

        SootClass targetClass = baseType.getSootClass();
        if (!SystemClassHandler.v().isClassInSystemPackage(targetClass.getName()))
            callbackClasses.add(targetClass);
    }
}

CHA is set up with a simple points-to analysis that only returns that locationListener can be "Any_subtype_of_android.location.LocationListener." In the for-loop, it only iterates through the base type (i.e., android.location.LocationListener) and not any of its subtypes (e.g., AnnonymousClass$1).