Changeset 90901 in webkit

Timestamp:

Jul 13, 2011, 4:33:53 AM (14 years ago)

Author:

Nikolas Zimmermann

Message:

2011-07-13 Nikolas Zimmermann <​nzimmermann@rim.com>

Regression: OOB read in svg text run
​https://bugs.webkit.org/show_bug.cgi?id=63627

Reviewed by Zoltan Herczeg.

A TextRun is constructed for a portion of a string [a,b] whose original length is c (0 < a < b < c).
The TextRun charactersLength variable stores the length of the remaining text from (b..c) in order
to support ligatures in SVG Fonts. Example: <text>ffl</text>. When measuring the advance from char 0
to char 1 the whole 'ffl' text must be passed to the SVG glyph selection code, as the SVG Font may
define a single glyph for the characters 'ffl' thus leading to a single character long text
pointing to the ffl ligature, not three individual 'f' / 'f' / 'l' characters anymore.

constructTextRun(..const UChar*, int length, ..) did not correctly calculate the maximum length (b..c).
The passed in UChar buffer starts at eg. textRenderer->characters() + start(), and following condition
holds true for 'length': start() + length <= textRenderer->textLength() (which denotes the maximum length
of the textRenderer->characters() buffer). We have to keep track of the start() offset, so that we
can calculate the charactersLength for the TextRun correctly: textRenderer->textLength() - start().

There are also other cases like RenderCombinedText and/or the presence of hyphens that were incorrectly
tracked. Only InlineTextBox had to be fixed, the other callsites in eg. RenderBlockLineLayout already
computed the maximum length correctly - I assured this by valgrind runs on all SVG Font tests.

• rendering/InlineTextBox.cpp: (WebCore::InlineTextBox::paint): (WebCore::InlineTextBox::paintSelection): (WebCore::InlineTextBox::constructTextRun): Add maximumLength parameter to constructTextRun.
• rendering/InlineTextBox.h: Ditto.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/InlineTextBox.cpp (modified) (5 diffs)
• rendering/InlineTextBox.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-07-13  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Regression: OOB read in svg text run
+        https://bugs.webkit.org/show_bug.cgi?id=63627
+
+        Reviewed by Zoltan Herczeg.
+
+        A TextRun is constructed for a portion of a string [a,b] whose original length is c (0 < a < b < c).
+        The TextRun charactersLength variable stores the length of the remaining text from (b..c) in order
+        to support ligatures in SVG Fonts. Example: <text>ffl</text>. When measuring the advance from char 0
+        to char 1 the whole 'ffl' text must be passed to the SVG glyph selection code, as the SVG Font may
+        define a single glyph for the characters 'ffl' thus leading to a single character long text
+        pointing to the ffl ligature, not three individual 'f' / 'f' / 'l' characters anymore.
+
+        constructTextRun(..const UChar*, int length, ..) did not correctly calculate the maximum length (b..c).
+        The passed in UChar buffer starts at eg. textRenderer->characters() + start(), and following condition
+        holds true for 'length': start() + length <= textRenderer->textLength() (which denotes the maximum length
+        of the textRenderer->characters() buffer). We have to keep track of the start() offset, so that we
+        can calculate the charactersLength for the TextRun correctly: textRenderer->textLength() - start().
+
+        There are also other cases like RenderCombinedText and/or the presence of hyphens that were incorrectly
+        tracked. Only InlineTextBox had to be fixed, the other callsites in eg. RenderBlockLineLayout already
+        computed the maximum length correctly - I assured this by valgrind runs on all SVG Font tests.
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint):
+        (WebCore::InlineTextBox::paintSelection):
+        (WebCore::InlineTextBox::constructTextRun): Add maximumLength parameter to constructTextRun.
+        * rendering/InlineTextBox.h: Ditto.
+
 2011-07-12  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/rendering/InlineTextBox.cpp

@@ -642 +642 @@
 
     int length = m_len;
+    int maximumLength;
     const UChar* characters;
-    if (!combinedText)
+    if (!combinedText) {
         characters = textRenderer()->text()->characters() + m_start;
-    else
+        maximumLength = textRenderer()->textLength() - m_start;
+    } else {
         combinedText->charactersToRender(m_start, characters, length);
+        maximumLength = length;
+    }
 
     BufferForAppendingHyphen charactersWithHyphen;
-    TextRun textRun = constructTextRun(styleToUse, font, characters, length, hasHyphen() ? &charactersWithHyphen : 0);
+    TextRun textRun = constructTextRun(styleToUse, font, characters, length, maximumLength, hasHyphen() ? &charactersWithHyphen : 0);
     if (hasHyphen())
         length = textRun.length();
@@ -814 +818 @@
     BufferForAppendingHyphen charactersWithHyphen;
     bool respectHyphen = ePos == length && hasHyphen();
-    TextRun textRun = constructTextRun(style, font, characters, length, respectHyphen ? &charactersWithHyphen : 0);
+    TextRun textRun = constructTextRun(style, font, characters, length, textRenderer()->textLength() - length, respectHyphen ? &charactersWithHyphen : 0);
     if (respectHyphen)
         ePos = textRun.length();
@@ -1299 +1303 @@
     ASSERT(textRenderer->characters());
 
-    return constructTextRun(style, font, textRenderer->characters() + start(), len(), charactersWithHyphen);
-}
-
-TextRun InlineTextBox::constructTextRun(RenderStyle* style, const Font& font, const UChar* characters, int length, BufferForAppendingHyphen* charactersWithHyphen) const
+    return constructTextRun(style, font, textRenderer->characters() + start(), len(), textRenderer->textLength() - start(), charactersWithHyphen);
+}
+
+TextRun InlineTextBox::constructTextRun(RenderStyle* style, const Font& font, const UChar* characters, int length, int maximumLength, BufferForAppendingHyphen* charactersWithHyphen) const
 {
     ASSERT(style);
@@ -1309 +1313 @@
     ASSERT(textRenderer);
 
-    if (charactersWithHyphen)
+    if (charactersWithHyphen) {
         adjustCharactersAndLengthForHyphen(*charactersWithHyphen, style, characters, length);
+        maximumLength = length;
+    }
+
+    ASSERT(maximumLength >= length);
 
     TextRun run(characters, length, textRenderer->allowTabs(), textPos(), expansion(), expansionBehavior(), direction(), m_dirOverride || style->rtlOrdering() == VisualOrder);
@@ -1317 +1325 @@
 
     // Propagate the maximum length of the characters buffer to the TextRun, even when we're only processing a substring.
-    run.setCharactersLength(textRenderer->textLength());
+    run.setCharactersLength(maximumLength);
+    ASSERT(run.charactersLength() >= run.length());
     return run;
 }

trunk/Source/WebCore/rendering/InlineTextBox.h

@@ -101 +101 @@
 
     TextRun constructTextRun(RenderStyle*, const Font&, BufferForAppendingHyphen* = 0) const;
-    TextRun constructTextRun(RenderStyle*, const Font&, const UChar*, int length, BufferForAppendingHyphen* = 0) const;
+    TextRun constructTextRun(RenderStyle*, const Font&, const UChar*, int length, int maximumLength, BufferForAppendingHyphen* = 0) const;
 
 public: