1172187 - Overflow in nsXMLContentSink::AddText causes memory-safety bug

Status: RESOLVED DUPLICATE of bug 890284

(Core :: XML, defect)

Description

[q1]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20150305021524

Steps to reproduce:

nsXMLContentSink::AddText can experience an overflow, causing a realloc to a tiny buffer size. The function then writes to unallocated space, potentially permitting the execution of attacker-chosen code, as by modifying an object's virtual-function table:

1436: nsresult
1437: nsXMLContentSink::AddText(const char16_t* aText,
1438:                           int32_t aLength)
1439: {
      ...
1448:   // Copy data from string into our buffer; flush buffer when it fills up
1449:   int32_t offset = 0;
1450:   while (0 != aLength) {
1451:     int32_t amount = mTextSize - mTextLength;
1452:     if (0 == amount) {
1453:       // XSLT wants adjacent textnodes merged.
1454:       if (mConstrainSize && !mXSLTProcessor) {
1455:         nsresult rv = FlushText();
1456:         if (NS_OK != rv) {
1457:           return rv;
1458:         }
1459:
1460:         amount = mTextSize - mTextLength;
1461:       }
1462:       else {
1463:         mTextSize += aLength;
1464:         mText = (char16_t *) PR_REALLOC(mText, sizeof(char16_t) * mTextSize);
1465:         if (nullptr == mText) {
1466:           mTextSize = 0;
1467: 
1468:           return NS_ERROR_OUT_OF_MEMORY;
1469:         }
1470:
1471:         amount = aLength;
1472:       }
1473:     }
1474:     if (amount > aLength) {
1475:       amount = aLength;
1476:     }
1477:     memcpy(&mText[mTextLength], &aText[offset], sizeof(char16_t) * amount);
1478:     mTextLength += amount;
1479:     offset += amount;
1480:     aLength -= amount;
1481:   }

The bug is in line 1463, which can overflow. Line 1464 then allocates a buffer that is too small to contain mTextSize + aLength characters, and line 1477 copies data beyond the buffer's end.

Comment 1

[q1]

https://bugzilla.mozilla.org/show_bug.cgi?id=651092 shows an apparently-dormant effort to remove this code from the project.

Comment 2

[Andrew McCreight [:mccr8]]

This realloc call was removed a few months later in bug 890284.