commit ccf543418d02ecae18275664c8aca1baab9367c5
author Prashant Kumar <pktrichy@codeaurora.org> Wed May 08 12:28:06 2019 +0530
committer Prashant Kumar <pktrichy@codeaurora.org> Wed May 08 15:36:33 2019 +0530
tree 60cd3984a3d701b43463765b215b8bce31b5f18e
parent 5903239bcb624279ec8011051927005a742df4c4

Integer overflow leading to a buffer overflow

Added a length check in to avoid integer overflow
in dataConnOpenCommand and set APN methods.
As the APN name is like few 100bytes so
using the micro defined int gps_extended_c.h.
Added typecast to avoid compiler errors in some
PLs

Change-Id: I2288c2c29e410571a72b92782b21f4b6c5ab4ff1
CRs-fixed: 2419292

msm8998/gnss/Agps.cpp

diff --git a/msm8998/gnss/Agps.cpp b/msm8998/gnss/Agps.cpp
index 32f3985..d88a6ba 100644
--- a/msm8998/gnss/Agps.cpp
+++ b/msm8998/gnss/Agps.cpp
@@ -454,15 +454,14 @@
 
     if (NULL != mAPN) {
         delete mAPN;
+        mAPN  = NULL;
     }
 
-    if(apn == NULL || len <= 0){
+    if(NULL == apn || len <= 0 || len > MAX_APN_LEN || strlen(apn) != len){
         LOC_LOGD("Invalid apn len (%d) or null apn", len);
         mAPN = NULL;
         mAPNLen = 0;
-    }
-
-    if (NULL != apn) {
+    }else{
         mAPN = new char[len+1];
         memcpy(mAPN, apn, len);
         mAPN[len] = '\0';

msm8998/gnss/GnssAdapter.cpp

diff --git a/msm8998/gnss/GnssAdapter.cpp b/msm8998/gnss/GnssAdapter.cpp
index 9809125..3386a63 100644
--- a/msm8998/gnss/GnssAdapter.cpp
+++ b/msm8998/gnss/GnssAdapter.cpp
@@ -2549,6 +2549,12 @@
                         new char[apnLen + 1]), mApnLen(apnLen), mIpType(ipType) {
 
             LOC_LOGV("AgpsMsgAtlOpenSuccess");
+            if (mApnName == nullptr) {
+                LOC_LOGE("%s] new allocation failed, fatal error.", __func__);
+                // Reporting the failure here
+                mAgpsManager->reportAtlClosed(mAgpsType);
+                return;
+            }
             memcpy(mApnName, apnName, apnLen);
             mApnName[apnLen] = 0;
         }
@@ -2565,8 +2571,16 @@
         }
     };
 
-    sendMsg( new AgpsMsgAtlOpenSuccess(
-            &mAgpsManager, (AGpsExtType)agpsType, apnName, apnLen, ipType));
+    // Added inital length checks for apnlen check to avoid security issues
+    // In case of failure reporting the same
+    if (NULL == apnName || apnLen <= 0 || apnLen > MAX_APN_LEN ||
+            (strlen(apnName) != (unsigned)apnLen)) {
+        LOC_LOGe("%s]: incorrect apnlen length or incorrect apnName", __func__);
+        mAgpsManager.reportAtlClosed(agpsType);
+    } else {
+        sendMsg( new AgpsMsgAtlOpenSuccess(
+                    &mAgpsManager, agpsType, apnName, apnLen, bearerType));
+    }
 }
 
 void GnssAdapter::dataConnClosedCommand(AGpsExtType agpsType){