On Sun, 2009-01-25 at 16:21 +0000, Ben Hutchings wrote:
[...]
> Ralink's Linux drivers are based on their Windows drivers and the
> following code in PeerProbeReqSanity() in the source file sanity.c
> appears to have exactly this flaw:
>
> if ((pFrame->Octet[0] != IE_SSID) || (pFrame->Octet[1] > MAX_LEN_OF_SSID))
> {
> DBGPRINT(RT_DEBUG_TRACE, "PeerProbeReqSanity fail - wrong SSID
> IE(Type=%d,Len=%d)\n",pFrame->Octet[0],pFrame->Octet[1]);
> return FALSE;
> }
>
> *pSsidLen = pFrame->Octet[1];
> memcpy(Ssid, &pFrame->Octet[2], *pSsidLen);
>
> pFrame->Octet is an array of signed char and MAX_LEN_OF_SSID expands
> to a decimal literal which will have type int. Therefore unsigned
> values in the range [128, 255] will be treated as values in the range
> [-128, -1] and will pass the test.
>
> Similar code exists in the rt2400, rt2500, rt2570, rt61 and rt2860
> drivers.
In the rt2860 driver pFrame->Octet is an array of unsigned char and so
the code appears to be correct. There is a similar bug in the handling
of IE_CF_PARM (also found in rt73). However I don't think it allows
code injection, and it might not be a security problem at all.
My proposed patch is:
--- rt2860-source-1.8.0.0.orig/common/cmm_sanity.c
+++ rt2860-source-1.8.0.0/common/cmm_sanity.c
@@ -517,8 +517,8 @@
pCfParm->bValid = TRUE;
pCfParm->CfpCount = pEid->Octet[0];
pCfParm->CfpPeriod = pEid->Octet[1];
- pCfParm->CfpMaxDuration = pEid->Octet[2] + 256 *
pEid->Octet[3];
- pCfParm->CfpDurRemaining = pEid->Octet[4] + 256 *
pEid->Octet[5];
+ pCfParm->CfpMaxDuration = (UCHAR)pEid->Octet[2] + 256 *
(UCHAR)pEid->Octet[3];
+ pCfParm->CfpDurRemaining = (UCHAR)pEid->Octet[4] + 256 *
(UCHAR)pEid->Octet[5];
}
else
{
--- END ---
Ben.
signature.asc
Description: This is a digitally signed message part
