wlan: DroidSec: String handling and input validation issues
Fix to address String handling and input validation issues
in function hdd_parse_reassoc_command_data and
hdd_parse_channellist
1. Input character buffer is zero terminated
2. Helper function is added to verify MAC address
3. return value of sscanf is checked
Change-Id: I352959c46488c1730ca35bf15f5f2d380ff3ac85
CRs-Fixed: 553494
diff --git a/CORE/HDD/src/wlan_hdd_main.c b/CORE/HDD/src/wlan_hdd_main.c
index 4a57141..a7afa2c 100644
--- a/CORE/HDD/src/wlan_hdd_main.c
+++ b/CORE/HDD/src/wlan_hdd_main.c
@@ -109,6 +109,7 @@
int wlan_hdd_ftm_start(hdd_context_t *pAdapter);
#include "sapApi.h"
#include <linux/semaphore.h>
+#include <linux/ctype.h>
#include <mach/subsystem_restart.h>
#include <wlan_hdd_hostapd.h>
#include <wlan_hdd_softap_tx_rx.h>
@@ -1522,7 +1523,17 @@
goto exit;
}
- command = kmalloc(priv_data.total_len, GFP_KERNEL);
+ if (priv_data.total_len <= 0)
+ {
+ VOS_TRACE(VOS_MODULE_ID_HDD, VOS_TRACE_LEVEL_WARN,
+ "%s:invalid priv_data.total_len(%d)!!!", __func__,
+ priv_data.total_len);
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ /* Allocate +1 for '\0' */
+ command = kmalloc(priv_data.total_len + 1, GFP_KERNEL);
if (!command)
{
VOS_TRACE( VOS_MODULE_ID_HDD, VOS_TRACE_LEVEL_FATAL,
@@ -1537,6 +1548,9 @@
goto exit;
}
+ /* Making sure the command is NUL-terminated */
+ command[priv_data.total_len] = '\0';
+
if ((SIOCDEVPRIVATE + 1) == cmd)
{
hdd_context_t *pHddCtx = (hdd_context_t*)pAdapter->pHddCtx;
@@ -3346,6 +3360,8 @@
int v = 0;
tANI_U8 tempBuf[32];
tANI_U8 tempByte = 0;
+ /* 12 hexa decimal digits and 5 ':' */
+ tANI_U8 macAddress[17];
inPtr = strnchr(pValue, strlen(pValue), SPACE_ASCII_VALUE);
/*no argument after the command*/
@@ -3369,14 +3385,20 @@
return -EINVAL;
}
- /*getting the first argument ie the target AP bssid */
- if (inPtr[2] != ':' || inPtr[5] != ':' || inPtr[8] != ':' || inPtr[11] != ':' || inPtr[14] != ':')
+ v = sscanf(inPtr, "%17s", macAddress);
+ if (!((1 == v) && hdd_is_valid_mac_address(macAddress)))
{
- return -EINVAL;
+ VOS_TRACE( VOS_MODULE_ID_HDD, VOS_TRACE_LEVEL_ERROR,
+ "Invalid MAC address or All hex inputs are not read (%d)", v);
+ return -EINVAL;
}
- j = sscanf(inPtr, "%2x:%2x:%2x:%2x:%2x:%2x", (unsigned int *)&pTargetApBssid[0], (unsigned int *)&pTargetApBssid[1],
- (unsigned int *)&pTargetApBssid[2], (unsigned int *)&pTargetApBssid[3],
- (unsigned int *)&pTargetApBssid[4], (unsigned int *)&pTargetApBssid[5]);
+
+ pTargetApBssid[0] = hdd_parse_hex(macAddress[0]) << 4 | hdd_parse_hex(macAddress[1]);
+ pTargetApBssid[1] = hdd_parse_hex(macAddress[3]) << 4 | hdd_parse_hex(macAddress[4]);
+ pTargetApBssid[2] = hdd_parse_hex(macAddress[6]) << 4 | hdd_parse_hex(macAddress[7]);
+ pTargetApBssid[3] = hdd_parse_hex(macAddress[9]) << 4 | hdd_parse_hex(macAddress[10]);
+ pTargetApBssid[4] = hdd_parse_hex(macAddress[12]) << 4 | hdd_parse_hex(macAddress[13]);
+ pTargetApBssid[5] = hdd_parse_hex(macAddress[15]) << 4 | hdd_parse_hex(macAddress[16]);
/* point to the next argument */
inPtr = strnchr(inPtr, strlen(inPtr), SPACE_ASCII_VALUE);
@@ -3393,7 +3415,9 @@
}
/*getting the next argument ie the channel number */
- j = sscanf(inPtr, "%32s ", tempBuf);
+ v = sscanf(inPtr, "%32s ", tempBuf);
+ if (1 != v) return -EINVAL;
+
v = kstrtos32(tempBuf, 10, &tempInt);
if ( v < 0) return -EINVAL;
@@ -3413,7 +3437,9 @@
}
/*getting the next argument ie the dwell time */
- j = sscanf(inPtr, "%32s ", tempBuf);
+ v = sscanf(inPtr, "%32s ", tempBuf);
+ if (1 != v) return -EINVAL;
+
v = kstrtos32(tempBuf, 10, &tempInt);
if ( v < 0) return -EINVAL;
@@ -3519,7 +3545,9 @@
}
/*getting the first argument ie the number of channels*/
- sscanf(inPtr, "%32s ", buf);
+ v = sscanf(inPtr, "%32s ", buf);
+ if (1 != v) return -EINVAL;
+
v = kstrtos32(buf, 10, &tempInt);
if ((v < 0) ||
(tempInt <= 0) ||
@@ -3568,7 +3596,9 @@
}
}
- sscanf(inPtr, "%32s ", buf);
+ v = sscanf(inPtr, "%32s ", buf);
+ if (1 != v) return -EINVAL;
+
v = kstrtos32(buf, 10, &tempInt);
if ((v < 0) ||
(tempInt <= 0) ||
@@ -3594,19 +3624,22 @@
This function parses the reasoc command data passed in the format
REASSOC<space><bssid><space><channel>
- \param - pValue Pointer to input data
+ \param - pValue Pointer to input data (its a NUL terminated string)
\param - pTargetApBssid Pointer to target Ap bssid
\param - pChannel Pointer to the Target AP channel
\return - 0 for success non-zero for failure
--------------------------------------------------------------------------*/
-VOS_STATUS hdd_parse_reassoc_command_data(tANI_U8 *pValue, tANI_U8 *pTargetApBssid, tANI_U8 *pChannel)
+VOS_STATUS hdd_parse_reassoc_command_data(tANI_U8 *pValue,
+ tANI_U8 *pTargetApBssid, tANI_U8 *pChannel)
{
tANI_U8 *inPtr = pValue;
int tempInt;
int v = 0;
tANI_U8 tempBuf[32];
+ /* 12 hexa decimal digits and 5 ':' */
+ tANI_U8 macAddress[17];
inPtr = strnchr(pValue, strlen(pValue), SPACE_ASCII_VALUE);
/*no argument after the command*/
@@ -3630,14 +3663,20 @@
return -EINVAL;
}
- /*getting the first argument ie the target AP bssid */
- if (inPtr[2] != ':' || inPtr[5] != ':' || inPtr[8] != ':' || inPtr[11] != ':' || inPtr[14] != ':')
+ v = sscanf(inPtr, "%17s", macAddress);
+ if (!((1 == v) && hdd_is_valid_mac_address(macAddress)))
{
- return -EINVAL;
+ VOS_TRACE( VOS_MODULE_ID_HDD, VOS_TRACE_LEVEL_ERROR,
+ "Invalid MAC address or All hex inputs are not read (%d)", v);
+ return -EINVAL;
}
- sscanf(inPtr, "%2x:%2x:%2x:%2x:%2x:%2x", (unsigned int *)&pTargetApBssid[0], (unsigned int *)&pTargetApBssid[1],
- (unsigned int *)&pTargetApBssid[2], (unsigned int *)&pTargetApBssid[3],
- (unsigned int *)&pTargetApBssid[4], (unsigned int *)&pTargetApBssid[5]);
+
+ pTargetApBssid[0] = hdd_parse_hex(macAddress[0]) << 4 | hdd_parse_hex(macAddress[1]);
+ pTargetApBssid[1] = hdd_parse_hex(macAddress[3]) << 4 | hdd_parse_hex(macAddress[4]);
+ pTargetApBssid[2] = hdd_parse_hex(macAddress[6]) << 4 | hdd_parse_hex(macAddress[7]);
+ pTargetApBssid[3] = hdd_parse_hex(macAddress[9]) << 4 | hdd_parse_hex(macAddress[10]);
+ pTargetApBssid[4] = hdd_parse_hex(macAddress[12]) << 4 | hdd_parse_hex(macAddress[13]);
+ pTargetApBssid[5] = hdd_parse_hex(macAddress[15]) << 4 | hdd_parse_hex(macAddress[16]);
/* point to the next argument */
inPtr = strnchr(inPtr, strlen(inPtr), SPACE_ASCII_VALUE);
@@ -3654,9 +3693,16 @@
}
/*getting the next argument ie the channel number */
- sscanf(inPtr, "%s ", tempBuf);
+ v = sscanf(inPtr, "%32s ", tempBuf);
+ if (1 != v) return -EINVAL;
+
v = kstrtos32(tempBuf, 10, &tempInt);
- if ( v < 0) return -EINVAL;
+ if ((v < 0) ||
+ (tempInt <= 0) ||
+ (tempInt > WNI_CFG_CURRENT_CHANNEL_STAMAX))
+ {
+ return -EINVAL;
+ }
*pChannel = tempInt;
return VOS_STATUS_SUCCESS;
@@ -3666,6 +3712,52 @@
/**---------------------------------------------------------------------------
+ \brief hdd_is_valid_mac_address() - Validate MAC address
+
+ This function validates whether the given MAC address is valid or not
+ Expected MAC address is of the format XX:XX:XX:XX:XX:XX
+ where X is the hexa decimal digit character and separated by ':'
+ This algorithm works even if MAC address is not separated by ':'
+
+ This code checks given input string mac contains exactly 12 hexadecimal digits.
+ and a separator colon : appears in the input string only after
+ an even number of hex digits.
+
+ \param - pMacAddr pointer to the input MAC address
+ \return - 1 for valid and 0 for invalid
+
+ --------------------------------------------------------------------------*/
+
+v_BOOL_t hdd_is_valid_mac_address(const tANI_U8 *pMacAddr)
+{
+ int xdigit = 0;
+ int separator = 0;
+ while (*pMacAddr)
+ {
+ if (isxdigit(*pMacAddr))
+ {
+ xdigit++;
+ }
+ else if (':' == *pMacAddr)
+ {
+ if (0 == xdigit || ((xdigit / 2) - 1) != separator)
+ break;
+
+ ++separator;
+ }
+ else
+ {
+ separator = -1;
+ /* Invalid MAC found */
+ return 0;
+ }
+ ++pMacAddr;
+ }
+ return (xdigit == 12 && (separator == 5 || separator == 0));
+}
+
+/**---------------------------------------------------------------------------
+
\brief hdd_open() - HDD Open function
This is called in response to ifconfig up