diff options
author | Shinru Han <shinruhan@google.com> | 2019-07-05 17:54:14 +0800 |
---|---|---|
committer | Shinru Han <shinruhan@google.com> | 2019-07-05 10:08:42 +0000 |
commit | d89dce95f10157544b8fed372050a89abda86b61 (patch) | |
tree | 95a14a0b2b1895e66c6ed2e28e09729b4bfbe062 | |
parent | 024d412c2e76607ba1ccfa12c75751b1e6f7bdd2 (diff) | |
download | gps-d89dce95f10157544b8fed372050a89abda86b61.tar.gz |
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
Test: Set the APN name with 150 characters in JNI method android_location_GnssLocationProvider_agps_data_conn_open.
Run SUPL test and confirm aps_data_conn_open is blocked by HAL.
Bug: 134437774
Change-Id: I5f6df32f88b00ae0e8d6690df595ea4618e224aa
-rw-r--r-- | msm8998/gnss/Agps.cpp | 15 | ||||
-rw-r--r-- | msm8998/gnss/GnssAdapter.cpp | 14 |
2 files changed, 21 insertions, 8 deletions
diff --git a/msm8998/gnss/Agps.cpp b/msm8998/gnss/Agps.cpp index e671daa..f2fcdd9 100644 --- a/msm8998/gnss/Agps.cpp +++ b/msm8998/gnss/Agps.cpp @@ -452,19 +452,20 @@ void AgpsStateMachine::setAPN(char* apn, unsigned int len){ 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'; - mAPNLen = len; + if (NULL != mAPN) { + memcpy(mAPN, apn, len); + mAPN[len] = '\0'; + mAPNLen = len; + } } } diff --git a/msm8998/gnss/GnssAdapter.cpp b/msm8998/gnss/GnssAdapter.cpp index fadf350..ac5f600 100644 --- a/msm8998/gnss/GnssAdapter.cpp +++ b/msm8998/gnss/GnssAdapter.cpp @@ -2543,6 +2543,12 @@ void GnssAdapter::dataConnOpenCommand( 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; } @@ -2558,9 +2564,15 @@ void GnssAdapter::dataConnOpenCommand( mIpType); } }; - + // 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) != apnLen)) { + LOC_LOGe("%s]: incorrect apnlen length or incorrect apnName", __func__); + mAgpsManager.reportAtlClosed(agpsType); + } else { sendMsg( new AgpsMsgAtlOpenSuccess( &mAgpsManager, (AGpsExtType)agpsType, apnName, apnLen, ipType)); + } } void GnssAdapter::dataConnClosedCommand(AGpsExtType agpsType){ |