summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorShinru Han <shinruhan@google.com>2019-07-05 17:54:14 +0800
committerShinru Han <shinruhan@google.com>2019-07-05 10:08:42 +0000
commitd89dce95f10157544b8fed372050a89abda86b61 (patch)
tree95a14a0b2b1895e66c6ed2e28e09729b4bfbe062
parent024d412c2e76607ba1ccfa12c75751b1e6f7bdd2 (diff)
downloadgps-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.cpp15
-rw-r--r--msm8998/gnss/GnssAdapter.cpp14
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){