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

VERIFIED INVALID

Status

()

Core
Networking
VERIFIED INVALID
14 years ago
14 years ago

People

(Reporter: Martin Gerbershagen, Assigned: Darin Fisher)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

14 years ago
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 ...
(Reporter)

Comment 1

14 years ago
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 ...
I don't see the problem. IsAsciiSpace returns PR_FALSE for 0.
Assignee: general → darin
Component: General → Networking
Product: Mozilla Application Suite → Core
QA Contact: general → benc
Version: unspecified → Trunk
(Reporter)

Comment 3

14 years ago
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.
(Reporter)

Comment 4

14 years ago
I checked it again. Please forget this report. I was wrong.
invalid per reporter.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → INVALID

Updated

14 years ago
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.