278842 - Potential out of bounds array access in netwerk/mime/src/nsMIMEHeaderParamImpl.cpp

Status: VERIFIED INVALID

(Core :: Networking, defect)

Description

[Martin Gerbershagen]

User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8a5) Gecko/20041129
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.8a5) Gecko/20041129

I found several pieces of the following code

    // Skip over whitespace, '=', and whitespace
    while (nsCRT::IsAsciiSpace(*str)) ++str;
    if (*str == '=') ++str;
    while (nsCRT::IsAsciiSpace(*str)) ++str;

in the function nsMIMEHeaderParamImpl::GetParameterInternal.
In the while condition a check for the terminating 0 byte of the string is 
missing. Also the 0 byte is not checked on nsCRT::IsAsciiSpace. If the 
input string contains only spaces until its end, the while loop can access
memory beyond the input string. This usually crashes the application or can
result in copying the contents of arbitrary memory to the result string and 
create a potential (maybe minor) security problem.

Reproducible: Always

Steps to Reproduce:



Expected Results:  
    while (*str && nsCRT::IsAsciiSpace(*str)) ++str;
    if (*str == '=') ++str;
    while (*str && nsCRT::IsAsciiSpace(*str)) ++str;
    if (!*str) return ...

Comment 1

[Martin Gerbershagen]

The checks for *str can be omitted in the suggested correction:

    while (nsCRT::IsAsciiSpace(*str)) ++str;
    if (*str == '=') ++str;
    while (&& nsCRT::IsAsciiSpace(*str)) ++str;
    if (!*str) return ...

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

I don't see the problem. IsAsciiSpace returns PR_FALSE for 0.

Comment 3

[Martin Gerbershagen]

The problem is in the else block in line 230 (marked with ->):



   // Skip over whitespace, '=', and whitespace
    while (nsCRT::IsAsciiSpace(*str)) ++str;
    if (*str == '=') ++str;
    while (nsCRT::IsAsciiSpace(*str)) ++str;

    if (*str != '"')
    {
      // The value is a token, not a quoted string.
      valueStart = str;
      for (valueEnd = str;
           *valueEnd && !nsCRT::IsAsciiSpace (*valueEnd) && *valueEnd != ';';
           valueEnd++)
        ;
      str = valueEnd;
    }
    else
    {
      // The value is a quoted string.
->    ++str;
      valueStart = str;
      for (valueEnd = str; *valueEnd; ++valueEnd)
      {
        if (*valueEnd == '\\')
          ++valueEnd;
        else if (*valueEnd == '"')
          break;
      }
      str = valueEnd + 1;
    }

If while (nsCRT::IsAsc...) terminates at the 0 byte, str is incremented and
the processing continues beyond the end of the string in the else block.

Comment 4

[Martin Gerbershagen]

I checked it again. Please forget this report. I was wrong.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

invalid per reporter.