diff options
| author | Nilesh Gharde <ngharde@codeaurora.org> | 2019-03-22 17:26:09 +0530 |
|---|---|---|
| committer | Gerrit - the friendly Code Review server <code-review@localhost> | 2019-03-27 07:02:19 -0700 |
| commit | b47ee496e588eab7d03cb2f3fb952d5fab1043ee (patch) | |
| tree | dc64c69157179dcd6b8d5714c799673b7e9df8c2 | |
| parent | 01869b4004179870db2160ed41283dce7fbbdcde (diff) | |
| download | gps-b47ee496e588eab7d03cb2f3fb952d5fab1043ee.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
Change-Id: Idb5ebbf2e3647de5fa07673f248c0c256d6c1b52
CRs-fixed: 2419292
| -rw-r--r-- | gnss/Agps.cpp | 7 | ||||
| -rw-r--r-- | gnss/GnssAdapter.cpp | 14 |
2 files changed, 14 insertions, 7 deletions
diff --git a/gnss/Agps.cpp b/gnss/Agps.cpp index a4f6a30..9de1329 100644 --- a/gnss/Agps.cpp +++ b/gnss/Agps.cpp @@ -445,15 +445,14 @@ 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]; if (NULL != mAPN) { memcpy(mAPN, apn, len); diff --git a/gnss/GnssAdapter.cpp b/gnss/GnssAdapter.cpp index f6dff89..8d171a6 100644 --- a/gnss/GnssAdapter.cpp +++ b/gnss/GnssAdapter.cpp @@ -3980,6 +3980,8 @@ void GnssAdapter::dataConnOpenCommand( 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); @@ -3996,9 +3998,15 @@ void GnssAdapter::dataConnOpenCommand( mAgpsManager->reportAtlOpenSuccess(mAgpsType, mApnName, mApnLen, mBearerType); } }; - - sendMsg( new AgpsMsgAtlOpenSuccess( - &mAgpsManager, agpsType, apnName, apnLen, bearerType)); + // 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, agpsType, apnName, apnLen, bearerType)); + } } void GnssAdapter::dataConnClosedCommand(AGpsExtType agpsType){ |