From 4990c799e80a5d01262630f541cb86aaa7727768 Mon Sep 17 00:00:00 2001 From: Sohani Rao Date: Fri, 17 Mar 2017 18:42:35 -0700 Subject: Protect NPD in WifiP2pServiceImpl Messages sent to the P2p state machine from an application or a system server can contain null objects. Ensure all external inputs to the service are checked for null before dereferncing. Bug: 36443767 Test: Unit test and test app Change-Id: I16434c1b29a58a75563a337e3b50c5c80b33e54f --- .../server/wifi/p2p/WifiP2pServiceImpl.java | 200 +++++++++++++++++---- 1 file changed, 168 insertions(+), 32 deletions(-) (limited to 'service') diff --git a/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java b/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java index 21ded6f11..983f6815a 100644 --- a/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java +++ b/service/java/com/android/server/wifi/p2p/WifiP2pServiceImpl.java @@ -716,8 +716,12 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { // it would have stopped regardless mDiscoveryPostponed = false; if (mDiscoveryBlocked) { + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } + StateMachine m = (StateMachine) message.obj; try { - StateMachine m = (StateMachine) message.obj; m.sendMessage(message.arg2); } catch (Exception e) { loge("unable to send BLOCK_DISCOVERY response: " + e); @@ -862,6 +866,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { break; case WifiP2pMonitor.P2P_GROUP_STARTED_EVENT: // unexpected group created, remove + if (message.obj == null) { + Log.e(TAG, "Illegal arguments"); + break; + } mGroup = (WifiP2pGroup) message.obj; loge("Unexpected group creation, remove " + mGroup); mWifiNative.p2pGroupRemove(mGroup.getInterface()); @@ -1142,8 +1150,12 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { mWifiNative.p2pFind(DISCOVER_TIMEOUT_S); } if (blocked) { + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } + StateMachine m = (StateMachine) message.obj; try { - StateMachine m = (StateMachine) message.obj; m.sendMessage(message.arg2); } catch (Exception e) { loge("unable to send BLOCK_DISCOVERY response: " + e); @@ -1197,12 +1209,20 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } break; case WifiP2pMonitor.P2P_DEVICE_FOUND_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } WifiP2pDevice device = (WifiP2pDevice) message.obj; if (mThisDevice.deviceAddress.equals(device.deviceAddress)) break; mPeers.updateSupplicantDetails(device); sendPeersChangedBroadcast(); break; case WifiP2pMonitor.P2P_DEVICE_LOST_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } device = (WifiP2pDevice) message.obj; // Gets current details for the one removed device = mPeers.remove(device.deviceAddress); @@ -1251,6 +1271,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { break; case WifiP2pMonitor.P2P_SERV_DISC_RESP_EVENT: if (DBG) logd(getName() + " receive service response"); + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } List sdRespList = (List) message.obj; for (WifiP2pServiceResponse resp : sdRespList) { @@ -1381,9 +1405,12 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { transitionTo(mUserAuthorizingNegotiationRequestState); break; case WifiP2pMonitor.P2P_INVITATION_RECEIVED_EVENT: + if (message.obj == null) { + Log.e(TAG, "Invalid argument(s)"); + break; + } WifiP2pGroup group = (WifiP2pGroup) message.obj; WifiP2pDevice owner = group.getOwner(); - if (owner == null) { int id = group.getNetworkId(); if (id < 0) { @@ -1400,10 +1427,8 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { break; } } - config = new WifiP2pConfig(); config.deviceAddress = group.getOwner().deviceAddress; - if (isConfigInvalid(config)) { loge("Dropping invitation request " + config); break; @@ -1412,7 +1437,7 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { // Check if we have the owner in peer list and use appropriate // wps method. Default is to use PBC. - if ((owner = mPeers.get(owner.deviceAddress)) != null) { + if (owner != null && ((owner = mPeers.get(owner.deviceAddress)) != null)) { if (owner.wpsPbcSupported()) { mSavedPeerConfig.wps.setup = WpsInfo.PBC; } else if (owner.wpsKeypadSupported()) { @@ -1460,6 +1485,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } break; case WifiP2pMonitor.P2P_GROUP_STARTED_EVENT: + if (message.obj == null) { + Log.e(TAG, "Invalid argument(s)"); + break; + } mGroup = (WifiP2pGroup) message.obj; if (DBG) logd(getName() + " group started"); @@ -1492,6 +1521,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { mWifiNative.p2pFlush(); break; case WifiP2pManager.SET_CHANNEL: + if (message.obj == null) { + Log.e(TAG, "Illegal arguments(s)"); + break; + } Bundle p2pChannels = (Bundle) message.obj; int lc = p2pChannels.getInt("lc", 0); int oc = p2pChannels.getInt("oc", 0); @@ -1562,6 +1595,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } break; case WifiP2pMonitor.P2P_DEVICE_LOST_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } WifiP2pDevice device = (WifiP2pDevice) message.obj; if (!mSavedPeerConfig.deviceAddress.equals(device.deviceAddress)) { if (DBG) { @@ -1687,14 +1724,20 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { @Override public boolean processMessage(Message message) { if (DBG) logd(getName() + message.toString()); - WifiP2pProvDiscEvent provDisc; - WifiP2pDevice device; + WifiP2pProvDiscEvent provDisc = null; + WifiP2pDevice device = null; switch (message.what) { case WifiP2pMonitor.P2P_PROV_DISC_PBC_RSP_EVENT: + if (message.obj == null) { + Log.e(TAG, "Invalid argument(s)"); + break; + } provDisc = (WifiP2pProvDiscEvent) message.obj; device = provDisc.device; - if (!device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) break; - + if (device != null + && !device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) { + break; + } if (mSavedPeerConfig.wps.setup == WpsInfo.PBC) { if (DBG) logd("Found a match " + mSavedPeerConfig); p2pConnectWithPinDisplay(mSavedPeerConfig); @@ -1702,10 +1745,16 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } break; case WifiP2pMonitor.P2P_PROV_DISC_ENTER_PIN_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } provDisc = (WifiP2pProvDiscEvent) message.obj; device = provDisc.device; - if (!device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) break; - + if (device != null + && !device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) { + break; + } if (mSavedPeerConfig.wps.setup == WpsInfo.KEYPAD) { if (DBG) logd("Found a match " + mSavedPeerConfig); // we already have the pin @@ -1719,10 +1768,19 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } break; case WifiP2pMonitor.P2P_PROV_DISC_SHOW_PIN_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } provDisc = (WifiP2pProvDiscEvent) message.obj; device = provDisc.device; - if (!device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) break; - + if (device == null) { + Log.e(TAG, "Invalid device"); + break; + } + if (!device.deviceAddress.equals(mSavedPeerConfig.deviceAddress)) { + break; + } if (mSavedPeerConfig.wps.setup == WpsInfo.DISPLAY) { if (DBG) logd("Found a match " + mSavedPeerConfig); mSavedPeerConfig.wps.pin = provDisc.pin; @@ -1760,9 +1818,12 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { if (DBG) logd(getName() + " go success"); break; case WifiP2pMonitor.P2P_GROUP_STARTED_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } mGroup = (WifiP2pGroup) message.obj; if (DBG) logd(getName() + " group started"); - if (mGroup.getNetworkId() == WifiP2pGroup.PERSISTENT_NET_ID) { // update cache information and set network id to mGroup. updatePersistentNetworks(NO_RELOAD); @@ -1988,10 +2049,16 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { @Override public boolean processMessage(Message message) { if (DBG) logd(getName() + message.toString()); + WifiP2pDevice device = null; + String deviceAddress = null; switch (message.what) { case WifiP2pMonitor.AP_STA_CONNECTED_EVENT: - WifiP2pDevice device = (WifiP2pDevice) message.obj; - String deviceAddress = device.deviceAddress; + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } + device = (WifiP2pDevice) message.obj; + deviceAddress = device.deviceAddress; // Clear timeout that was set when group was started. mWifiNative.setP2pGroupIdle(mGroup.getInterface(), 0); if (deviceAddress != null) { @@ -2009,6 +2076,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { sendP2pConnectionChangedBroadcast(); break; case WifiP2pMonitor.AP_STA_DISCONNECTED_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + break; + } device = (WifiP2pDevice) message.obj; deviceAddress = device.deviceAddress; if (deviceAddress != null) { @@ -2048,12 +2119,16 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { break; case IPM_PROVISIONING_SUCCESS: if (DBG) logd("mDhcpResults: " + mDhcpResults); - setWifiP2pInfoOnGroupFormation(mDhcpResults.serverAddress); + if (mDhcpResults != null) { + setWifiP2pInfoOnGroupFormation(mDhcpResults.serverAddress); + } sendP2pConnectionChangedBroadcast(); try { final String ifname = mGroup.getInterface(); - mNwService.addInterfaceToLocalNetwork( - ifname, mDhcpResults.getRoutes(ifname)); + if (mDhcpResults != null) { + mNwService.addInterfaceToLocalNetwork( + ifname, mDhcpResults.getRoutes(ifname)); + } } catch (RemoteException e) { loge("Failed to add iface to local network " + e); } @@ -2090,16 +2165,20 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { transitionTo(mInactiveState); break; case WifiP2pMonitor.P2P_DEVICE_LOST_EVENT: + if (message.obj == null) { + Log.e(TAG, "Illegal argument(s)"); + return NOT_HANDLED; + } device = (WifiP2pDevice) message.obj; + if (!mGroup.contains(device)) { + // do the regular device lost handling + return NOT_HANDLED; + } // Device loss for a connected device indicates // it is not in discovery any more - if (mGroup.contains(device)) { - if (DBG) logd("Add device to lost list " + device); - mPeersLostDuringConnection.updateSupplicantDetails(device); - return HANDLED; - } - // Do the regular device lost handling - return NOT_HANDLED; + if (DBG) logd("Add device to lost list " + device); + mPeersLostDuringConnection.updateSupplicantDetails(device); + return HANDLED; case WifiStateMachine.CMD_DISABLE_P2P_REQ: sendMessage(WifiP2pManager.REMOVE_GROUP); deferMessage(message); @@ -2136,7 +2215,7 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { case WifiP2pManager.CONNECT: WifiP2pConfig config = (WifiP2pConfig) message.obj; if (isConfigInvalid(config)) { - loge("Dropping connect requeset " + config); + loge("Dropping connect request " + config); replyToMessage(message, WifiP2pManager.CONNECT_FAILED); break; } @@ -2182,7 +2261,9 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { case WifiP2pMonitor.P2P_PROV_DISC_SHOW_PIN_EVENT: WifiP2pProvDiscEvent provDisc = (WifiP2pProvDiscEvent) message.obj; mSavedPeerConfig = new WifiP2pConfig(); - mSavedPeerConfig.deviceAddress = provDisc.device.deviceAddress; + if (provDisc != null && provDisc.device != null) { + mSavedPeerConfig.deviceAddress = provDisc.device.deviceAddress; + } if (message.what == WifiP2pMonitor.P2P_PROV_DISC_ENTER_PIN_EVENT) { mSavedPeerConfig.wps.setup = WpsInfo.KEYPAD; } else if (message.what == WifiP2pMonitor.P2P_PROV_DISC_SHOW_PIN_EVENT) { @@ -2561,6 +2642,7 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private WifiP2pDevice fetchCurrentDeviceDetails(WifiP2pConfig config) { + if (config == null) return null; // Fetch & update group capability from supplicant on the device int gc = mWifiNative.getGroupCapability(config.deviceAddress); // TODO: The supplicant does not provide group capability changes as an event. @@ -2575,8 +2657,15 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { * @param config for the peer */ private void p2pConnectWithPinDisplay(WifiP2pConfig config) { + if (config == null) { + Log.e(TAG, "Illegal argument(s)"); + return; + } WifiP2pDevice dev = fetchCurrentDeviceDetails(config); - + if (dev == null) { + Log.e(TAG, "Invalid device"); + return; + } String pin = mWifiNative.p2pConnect(config, dev.isGroupOwner()); try { Integer.parseInt(pin); @@ -2593,8 +2682,15 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { * @return true on success, false on failure */ private boolean reinvokePersistentGroup(WifiP2pConfig config) { + if (config == null) { + Log.e(TAG, "Illegal argument(s)"); + return false; + } WifiP2pDevice dev = fetchCurrentDeviceDetails(config); - + if (dev == null) { + Log.e(TAG, "Invalid device"); + return false; + } boolean join = dev.isGroupOwner(); String ssid = mWifiNative.p2pGetSsid(dev.deviceAddress); if (DBG) logd("target ssid is " + ssid + " join:" + join); @@ -2986,9 +3082,14 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private boolean addServiceRequest(Messenger m, WifiP2pServiceRequest req) { + if (m == null || req == null) { + Log.e(TAG, "Illegal argument(s)"); + return false; + } // TODO: We could track individual service adds separately and avoid // having to do update all service requests on every new request clearClientDeadChannels(); + ClientInfo clientInfo = getClientInfo(m, true); if (clientInfo == null) { return false; @@ -3008,6 +3109,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private void removeServiceRequest(Messenger m, WifiP2pServiceRequest req) { + if (m == null || req == null) { + Log.e(TAG, "Illegal argument(s)"); + } + ClientInfo clientInfo = getClientInfo(m, false); if (clientInfo == null) { return; @@ -3039,6 +3144,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private void clearServiceRequests(Messenger m) { + if (m == null) { + Log.e(TAG, "Illegal argument(s)"); + return; + } ClientInfo clientInfo = getClientInfo(m, false); if (clientInfo == null) { @@ -3064,8 +3173,15 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private boolean addLocalService(Messenger m, WifiP2pServiceInfo servInfo) { + if (m == null || servInfo == null) { + Log.e(TAG, "Illegal arguments"); + return false; + } + clearClientDeadChannels(); + ClientInfo clientInfo = getClientInfo(m, true); + if (clientInfo == null) { return false; } @@ -3083,14 +3199,19 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private void removeLocalService(Messenger m, WifiP2pServiceInfo servInfo) { + if (m == null || servInfo == null) { + Log.e(TAG, "Illegal arguments"); + return; + } + ClientInfo clientInfo = getClientInfo(m, false); if (clientInfo == null) { return; } mWifiNative.p2pServiceDel(servInfo); - clientInfo.mServList.remove(servInfo); + if (clientInfo.mReqList.size() == 0 && clientInfo.mServList.size() == 0) { if (DBG) logd("remove client information from framework"); mClientInfoList.remove(clientInfo.mMessenger); @@ -3098,6 +3219,11 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { } private void clearLocalServices(Messenger m) { + if (m == null) { + Log.e(TAG, "Illegal argument(s)"); + return; + } + ClientInfo clientInfo = getClientInfo(m, false); if (clientInfo == null) { return; @@ -3124,6 +3250,10 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { * @param WifiP2pServiceResponse response to service discovery */ private void sendServiceResponse(WifiP2pServiceResponse resp) { + if (resp == null) { + Log.e(TAG, "sendServiceResponse with null response"); + return; + } for (ClientInfo c : mClientInfoList.values()) { WifiP2pServiceRequest req = c.mReqList.get(resp.getTransactionId()); if (req != null) { @@ -3132,6 +3262,9 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { msg.arg1 = 0; msg.arg2 = 0; msg.obj = resp; + if (c.mMessenger == null) { + continue; + } try { c.mMessenger.send(msg); } catch (RemoteException e) { @@ -3159,6 +3292,9 @@ public class WifiP2pServiceImpl extends IWifiP2pManager.Stub { msg.arg1 = 0; msg.arg2 = 0; msg.obj = null; + if (c.mMessenger == null) { + continue; + } try { c.mMessenger.send(msg); } catch (RemoteException e) { -- cgit v1.2.3