Merge "Fix the issue an invalid ScanResult may cause crash" into rvc-dev

4 files changed, 140 insertions, 14 deletions

diff --git a/service/java/com/android/server/wifi/WifiServiceImpl.java b/service/java/com/android/server/wifi/WifiServiceImpl.java
index 4e07e5ef3..e6a12c16c 100644
--- a/service/java/com/android/server/wifi/WifiServiceImpl.java
+++ b/service/java/com/android/server/wifi/WifiServiceImpl.java
@@ -115,6 +115,7 @@ import com.android.server.wifi.hotspot2.PasspointProvider;
 import com.android.server.wifi.util.ApConfigUtil;
 import com.android.server.wifi.util.ExternalCallbackTracker;
 import com.android.server.wifi.util.RssiUtil;
+import com.android.server.wifi.util.ScanResultUtil;
 import com.android.server.wifi.util.WifiHandler;
 import com.android.server.wifi.util.WifiPermissionsUtil;
 import com.android.wifi.resources.R;
@@ -2194,6 +2195,10 @@ public class WifiServiceImpl extends BaseWifiService {
         if (mVerboseLoggingEnabled) {
             mLog.info("getMatchingPasspointConfigurations uid=%").c(Binder.getCallingUid()).flush();
         }
+        if (!ScanResultUtil.validateScanResultList(scanResults)) {
+            Log.e(TAG, "Attempt to retrieve passpoint with invalid scanResult List");
+            return Collections.emptyMap();
+        }
         return mWifiThreadRunner.call(
             () -> mPasspointManager.getAllMatchingPasspointProfilesForScanResults(scanResults),
                 Collections.emptyMap());
@@ -2214,6 +2219,11 @@ public class WifiServiceImpl extends BaseWifiService {
         if (mVerboseLoggingEnabled) {
             mLog.info("getMatchingOsuProviders uid=%").c(Binder.getCallingUid()).flush();
         }
+
+        if (!ScanResultUtil.validateScanResultList(scanResults)) {
+            Log.e(TAG, "Attempt to retrieve OsuProviders with invalid scanResult List");
+            return Collections.emptyMap();
+        }
         return mWifiThreadRunner.call(
             () -> mPasspointManager.getMatchingOsuProviders(scanResults), Collections.emptyMap());
     }
@@ -2290,8 +2300,8 @@ public class WifiServiceImpl extends BaseWifiService {
             mLog.info("getWifiConfigsForMatchedNetworkSuggestions uid=%").c(
                     Binder.getCallingUid()).flush();
         }
-        if (scanResults == null) {
-            Log.e(TAG, "Attempt to retrieve WifiConfiguration with null scanResult List");
+        if (!ScanResultUtil.validateScanResultList(scanResults)) {
+            Log.e(TAG, "Attempt to retrieve WifiConfiguration with invalid scanResult List");
             return new ArrayList<>();
         }
         return mWifiThreadRunner.call(
@@ -2703,7 +2713,7 @@ public class WifiServiceImpl extends BaseWifiService {
 
             return mWifiThreadRunner.call(
                     () -> {
-                        if (scanResults == null || scanResults.isEmpty()) {
+                        if (!ScanResultUtil.validateScanResultList(scanResults)) {
                             return mWifiNetworkSuggestionsManager.getMatchingScanResults(
                                     networkSuggestions, mScanRequestProxy.getScanResults());
                         } else {
diff --git a/service/java/com/android/server/wifi/util/ScanResultUtil.java b/service/java/com/android/server/wifi/util/ScanResultUtil.java
index 320490c0b..dc2281ad2 100644
--- a/service/java/com/android/server/wifi/util/ScanResultUtil.java
+++ b/service/java/com/android/server/wifi/util/ScanResultUtil.java
@@ -243,4 +243,24 @@ public class ScanResultUtil {
             }
         }
     }
+
+    /**
+     * Check if ScarResult list is valid.
+     */
+    public static boolean validateScanResultList(List<ScanResult> scanResults) {
+        if (scanResults == null || scanResults.isEmpty()) {
+            return false;
+        }
+        for (ScanResult scanResult : scanResults) {
+            if (!validate(scanResult)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private static boolean validate(ScanResult scanResult) {
+        return scanResult != null && scanResult.SSID != null
+                && scanResult.capabilities != null && scanResult.BSSID != null;
+    }
 }
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
index 8286f1dc1..ca8c9dbc6 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiServiceImplTest.java
@@ -174,6 +174,7 @@ import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -225,6 +226,12 @@ public class WifiServiceImplTest extends WifiBaseTest {
     private static final int TEST_AP_FREQUENCY = 2412;
     private static final int TEST_AP_BANDWIDTH = SoftApInfo.CHANNEL_WIDTH_20MHZ;
     private static final int NETWORK_CALLBACK_ID = 1100;
+    private static final String TEST_CAP = "[RSN-PSK-CCMP]";
+    private static final String TEST_SSID = "Sid's Place";
+    private static final String TEST_SSID_WITH_QUOTES = "\"" + TEST_SSID + "\"";
+    private static final String TEST_BSSID = "01:02:03:04:05:06";
+    private static final String TEST_PACKAGE = "package";
+    private static final int TEST_NETWORK_ID = 567;
 
     private SoftApInfo mTestSoftApInfo;
     private AsyncChannel mAsyncChannel;
@@ -1558,12 +1565,6 @@ public class WifiServiceImplTest extends WifiBaseTest {
         verify(mScanRequestProxy).startScan(Binder.getCallingUid(), SCAN_PACKAGE_NAME);
     }
 
-    static final String TEST_SSID = "Sid's Place";
-    static final String TEST_SSID_WITH_QUOTES = "\"" + TEST_SSID + "\"";
-    static final String TEST_BSSID = "01:02:03:04:05:06";
-    static final String TEST_PACKAGE = "package";
-    static final int TEST_NETWORK_ID = 567;
-
     private void setupForGetConnectionInfo() {
         WifiInfo wifiInfo = new WifiInfo();
         wifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(TEST_SSID));
@@ -3138,9 +3139,9 @@ public class WifiServiceImplTest extends WifiBaseTest {
     }
 
     /**
-     * Verify that the call to getAllMatchingFqdnsForScanResults is not redirected to specific API
-     * syncGetAllMatchingFqdnsForScanResults when the caller doesn't have NETWORK_SETTINGS
-     * permissions and NETWORK_SETUP_WIZARD.
+     * Verify that the call to getAllMatchingPasspointProfilesForScanResults is not redirected to
+     * specific API getAllMatchingPasspointProfilesForScanResults when the caller doesn't have
+     * NETWORK_SETTINGS permissions and NETWORK_SETUP_WIZARD.
      */
     @Test(expected = SecurityException.class)
     public void testGetAllMatchingPasspointProfilesForScanResultsWithoutPermissions() {
@@ -3148,6 +3149,34 @@ public class WifiServiceImplTest extends WifiBaseTest {
     }
 
     /**
+     * Verify that the call to getAllMatchingPasspointProfilesForScanResults is redirected to
+     * specific API getAllMatchingPasspointProfilesForScanResults when the caller have
+     * NETWORK_SETTINGS permissions and NETWORK_SETUP_WIZARD.
+     */
+    @Test
+    public void testGetAllMatchingPasspointProfilesForScanResultsWithPermissions() {
+        when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
+                anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
+        mWifiServiceImpl.getAllMatchingPasspointProfilesForScanResults(createScanResultList());
+        mLooper.dispatchAll();
+        verify(mPasspointManager).getAllMatchingPasspointProfilesForScanResults(any());
+    }
+
+    /**
+     * Verify that the call to getAllMatchingPasspointProfilesForScanResults is not redirected to
+     * specific API getAllMatchingPasspointProfilesForScanResults when the caller provider invalid
+     * ScanResult.
+     */
+    @Test
+    public void testGetAllMatchingPasspointProfilesForScanResultsWithInvalidScanResult() {
+        when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
+                anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
+        mWifiServiceImpl.getAllMatchingPasspointProfilesForScanResults(new ArrayList<>());
+        mLooper.dispatchAll();
+        verify(mPasspointManager, never()).getAllMatchingPasspointProfilesForScanResults(any());
+    }
+
+    /**
      * Verify that the call to getWifiConfigsForPasspointProfiles is not redirected to specific API
      * syncGetWifiConfigsForPasspointProfiles when the caller doesn't have NETWORK_SETTINGS
      * permissions and NETWORK_SETUP_WIZARD.
@@ -3168,6 +3197,33 @@ public class WifiServiceImplTest extends WifiBaseTest {
     }
 
     /**
+     * Verify that the call to getMatchingOsuProviders is redirected to specific API
+     * syncGetMatchingOsuProviders when the caller have NETWORK_SETTINGS
+     * permissions and NETWORK_SETUP_WIZARD.
+     */
+    @Test
+    public void testGetMatchingOsuProvidersWithPermissions() {
+        when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
+                anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
+        mWifiServiceImpl.getMatchingOsuProviders(createScanResultList());
+        mLooper.dispatchAll();
+        verify(mPasspointManager).getMatchingOsuProviders(any());
+    }
+
+    /**
+     * Verify that the call to getMatchingOsuProviders is not redirected to specific API
+     * syncGetMatchingOsuProviders when the caller provider invalid ScanResult
+     */
+    @Test
+    public void testGetMatchingOsuProvidersWithInvalidScanResult() {
+        when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
+                anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
+        mWifiServiceImpl.getMatchingOsuProviders(new ArrayList<>());
+        mLooper.dispatchAll();
+        verify(mPasspointManager, never()).getMatchingOsuProviders(any());
+    }
+
+    /**
      * Verify that the call to getMatchingPasspointConfigsForOsuProviders is not redirected to
      * specific API syncGetMatchingPasspointConfigsForOsuProviders when the caller doesn't have
      * NETWORK_SETTINGS permissions and NETWORK_SETUP_WIZARD.
@@ -5372,7 +5428,8 @@ public class WifiServiceImplTest extends WifiBaseTest {
     public void testGetWifiConfigsForMatchedNetworkSuggestionsWithSettingPermissions() {
         when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
                 anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
-        mWifiServiceImpl.getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(new ArrayList<>());
+        mWifiServiceImpl
+                .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(createScanResultList());
         mLooper.dispatchAll();
         verify(mWifiNetworkSuggestionsManager)
                 .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(any());
@@ -5387,12 +5444,24 @@ public class WifiServiceImplTest extends WifiBaseTest {
     public void testGetWifiConfigsForMatchedNetworkSuggestionsWithSetupWizardPermissions() {
         when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETUP_WIZARD),
                 anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
-        mWifiServiceImpl.getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(new ArrayList<>());
+        mWifiServiceImpl
+                .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(createScanResultList());
         mLooper.dispatchAll();
         verify(mWifiNetworkSuggestionsManager)
                 .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(any());
     }
 
+    @Test
+    public void testGetWifiConfigsForMatchedNetworkSuggestionsWithInvalidScanResults() {
+        when(mContext.checkPermission(eq(android.Manifest.permission.NETWORK_SETTINGS),
+                anyInt(), anyInt())).thenReturn(PackageManager.PERMISSION_GRANTED);
+        mWifiServiceImpl
+                .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(new ArrayList<>());
+        mLooper.dispatchAll();
+        verify(mWifiNetworkSuggestionsManager, never())
+                .getWifiConfigForMatchedNetworkSuggestionsSharedWithUser(any());
+    }
+
     /**
      * Verify that a call to setWifiConnectedNetworkScorer throws a SecurityException if
      * the caller does not have WIFI_UPDATE_USABILITY_STATS_SCORE permission.
@@ -5670,4 +5739,9 @@ public class WifiServiceImplTest extends WifiBaseTest {
         verify(mSettingsStore, never()).handleWifiScanAlwaysAvailableToggled(anyBoolean());
         verify(mActiveModeWarden, never()).scanAlwaysModeChanged();
     }
+
+    private List<ScanResult> createScanResultList() {
+        return Collections.singletonList(new ScanResult(WifiSsid.createFromAsciiEncoded(TEST_SSID),
+                TEST_SSID, TEST_BSSID, 1245, 0, TEST_CAP, -78, 2450, 1025, 22, 33, 20, 0, 0, true));
+    }
 }
diff --git a/tests/wifitests/src/com/android/server/wifi/util/ScanResultUtilTest.java b/tests/wifitests/src/com/android/server/wifi/util/ScanResultUtilTest.java
index 0f69316e7..e58d083c4 100644
--- a/tests/wifitests/src/com/android/server/wifi/util/ScanResultUtilTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/util/ScanResultUtilTest.java
@@ -31,7 +31,9 @@ import com.android.server.wifi.WifiBaseTest;
 import org.junit.Test;
 
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.List;
 
 /**
  * Unit tests for {@link com.android.server.wifi.util.ScanResultUtil}.
@@ -243,6 +245,26 @@ public class ScanResultUtilTest extends WifiBaseTest {
         assertTrue(ScanResultUtil.isScanResultForFilsSha384Network(input));
     }
 
+    /**
+     * Verify ScanResultList validation.
+     */
+    @Test
+    public void testValidateScanResultList() {
+        List<ScanResult> scanResults = new ArrayList<>();
+        assertFalse(ScanResultUtil.validateScanResultList(null));
+        assertFalse(ScanResultUtil.validateScanResultList(scanResults));
+        scanResults.add(null);
+        assertFalse(ScanResultUtil.validateScanResultList(scanResults));
+        ScanResult scanResult = new ScanResult();
+        scanResults.clear();
+        scanResults.add(scanResult);
+        assertFalse(ScanResultUtil.validateScanResultList(scanResults));
+        scanResult.SSID = "test";
+        scanResult.capabilities = "[RSN-PSK-CCMP]";
+        scanResult.BSSID = "ab:cd:01:ef:45:89";
+        assertTrue(ScanResultUtil.validateScanResultList(scanResults));
+    }
+
     private static InformationElement createIE(int id, byte[] bytes) {
         InformationElement ie = new InformationElement();
         ie.id = id;