From a49af7fd6bf5d51ef7c34fbf2d71ee10c0f7d1dd Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 1 Aug 2017 17:08:40 -0700 Subject: WifiStateMachine: Always return a copy of WifiInfo WifiStateMachine#syncRequestConnectionInfo() should always return a copy of WifiInfo to prevent external services directly accessing a local variable in WifiStateMachine. Binder calls within the same process doesn't parcel objects, hence this is needed to prevent other threads in system_server directly using a local object within the wifi service. Bug: 64207440 Test: Unit test that checks for object returned from WifiStateMachine#getWifiInfo() vs WifiStateMachine#syncRequestConnectionInfo(). Change-Id: I3d709a09d3d10a1d133fd79eeee430830eb008b2 --- .../java/com/android/server/wifi/WifiStateMachine.java | 4 ++-- .../com/android/server/wifi/WifiStateMachineTest.java | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java index efea94b78..5f970026f 100644 --- a/service/java/com/android/server/wifi/WifiStateMachine.java +++ b/service/java/com/android/server/wifi/WifiStateMachine.java @@ -1758,9 +1758,9 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss */ public WifiInfo syncRequestConnectionInfo() { int uid = Binder.getCallingUid(); - if (uid == Process.myUid()) return mWifiInfo; - boolean hideBssidAndSsid = true; WifiInfo result = new WifiInfo(mWifiInfo); + if (uid == Process.myUid()) return result; + boolean hideBssidAndSsid = true; result.setMacAddress(WifiInfo.DEFAULT_MAC_ADDRESS); IPackageManager packageManager = AppGlobals.getPackageManager(); diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java index 631ec3f4c..7d9f6e655 100644 --- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java +++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java @@ -1791,20 +1791,30 @@ public class WifiStateMachineTest { } /** - * Test that the process uid has full wifiInfo access + * Test that the process uid has full wifiInfo access. + * Also tests that {@link WifiStateMachine#syncRequestConnectionInfo()} always + * returns a copy of WifiInfo. */ @Test public void testConnectedIdsAreVisibleFromOwnUid() throws Exception { assertEquals(Process.myUid(), Binder.getCallingUid()); WifiInfo wifiInfo = mWsm.getWifiInfo(); + wifiInfo.setBSSID(sBSSID); + wifiInfo.setSSID(WifiSsid.createFromAsciiEncoded(sSSID)); + connect(); WifiInfo connectionInfo = mWsm.syncRequestConnectionInfo(); - assertEquals(wifiInfo, connectionInfo); + + assertNotEquals(wifiInfo, connectionInfo); + assertEquals(wifiInfo.getSSID(), connectionInfo.getSSID()); + assertEquals(wifiInfo.getBSSID(), connectionInfo.getBSSID()); } /** * Test that connected SSID and BSSID are not exposed to an app that does not have the * appropriate permissions. + * Also tests that {@link WifiStateMachine#syncRequestConnectionInfo()} always + * returns a copy of WifiInfo. */ @Test public void testConnectedIdsAreHiddenFromRandomApp() throws Exception { @@ -1836,6 +1846,8 @@ public class WifiStateMachineTest { /** * Test that connected SSID and BSSID are not exposed to an app that does not have the * appropriate permissions, when canAccessScanResults raises a SecurityException. + * Also tests that {@link WifiStateMachine#syncRequestConnectionInfo()} always + * returns a copy of WifiInfo. */ @Test public void testConnectedIdsAreHiddenOnSecurityException() throws Exception { -- cgit v1.2.3