[Webkit-unassigned] [Bug 60771] New: Integer overflow on chrome in text width estimation code

Fri May 13 08:12:43 PDT 2011

https://bugs.webkit.org/show_bug.cgi?id=60771

           Summary: Integer overflow on chrome in text width estimation
                    code
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Windows Vista
            Status: NEW
          Severity: Normal
          Priority: P1
         Component: CSS
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: skylined at chromium.org

Created an attachment (id=93454)
 --> (https://bugs.webkit.org/attachment.cgi?id=93454&action=review)
Repro (crashes renderer)

Chromium bug: http://code.google.com/p/chromium/issues/detail?id=82275
Repro:
<html>
  <script>
    iFontSize = 0x7300;
    iLetterSpacing = 0x1376;
    iStringLength = 0xFFFF;
    setTimeout(function f() {
      document.write(
        '<style>*{' +
          'font-size: ' + iFontSize + 'px;' + 
          'letter-spacing: ' + iLetterSpacing + 'px;' +
          '-webkit-box-reflect: above -1px url(x);' +
          '-webkit-transition-duration: 0s;' +
        '}</style>' +
        '<body>' + new Array(iStringLength + 1).join('W') + '</body>');
    }, 1000);
  </script>
</html>

webkit\source\webcore\platform\graphics\chromium\fontchromiumwin.cpp @ 244:
IntRect TransparencyAwareGlyphPainter::estimateTextBounds()
{
    int totalWidth = 0;
    for (int i = 0; i < m_numGlyphs; i++)
        totalWidth += lroundf(m_glyphBuffer.advanceAt(m_from + i));

    const FontMetrics& fontMetrics = m_font->fontMetrics();
    return IntRect(m_point.x() - (fontMetrics.ascent() + fontMetrics.descent()) / 2,
                   m_point.y() - fontMetrics.ascent() - fontMetrics.lineGap(),
                   totalWidth + fontMetrics.ascent() + fontMetrics.descent(),
                   fontMetrics.lineSpacing()); 
}

The repro cause the value of the integer totalWidth to get incremented to a very large number, which combined with the "ascent" and "descent" values will overflow. The result is a very large negative width for the IntRect.

A little later one in code flow, it looks like the code tries to check if the values make sense:
bad math related to "pixelSize":
c:\src\chromium\src\third_party\webkit\source\webcore\platform\graphics\chromium\transparencywin.cpp @ 358:
void TransparencyWin::initializeNewContext()
{
    int pixelSize = m_layerSize.width() * m_layerSize.height();
    if (pixelSize <= 0) 
        return;

Unfortunately, this check is as good as useless and we do not return. On windows, the negative width makes it all way up to the platform layer where GDI returns an error when the code asks windows to allocate a bitmap. The code detects this failure and terminates the renderer as if there had been an OOM condition.

It should be obvious that the math for totalWidth and IntRect width and the sanity checks on the IntRects width are incorrect and that we should not rely on the platform layer to save us.

I was hoping I could find a way to increase totalWidth to the point where the value becomes positive again, so GDI doesn't throw an error and I could prove this is actually exploitable. I have been unable to do this because the css properties I am using seem to get ignored if I increase them any further. There must be some earlier checks, but I was unable to find them.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.