Modified: trunk/Source/WebCore/ChangeLog (90900 => 90901)
--- trunk/Source/WebCore/ChangeLog 2011-07-13 10:23:54 UTC (rev 90900)
+++ trunk/Source/WebCore/ChangeLog 2011-07-13 11:33:53 UTC (rev 90901)
@@ -1,3 +1,33 @@
+2011-07-13 Nikolas Zimmermann <[email protected]>
+
+ 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 <[email protected]>
didFirstVisuallyNonEmptyLayout dispatched too early
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (90900 => 90901)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2011-07-13 10:23:54 UTC (rev 90900)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2011-07-13 11:33:53 UTC (rev 90901)
@@ -641,14 +641,18 @@
}
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();
@@ -813,7 +817,7 @@
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();
@@ -1298,25 +1302,30 @@
ASSERT(textRenderer);
ASSERT(textRenderer->characters());
- return constructTextRun(style, font, textRenderer->characters() + start(), len(), charactersWithHyphen);
+ 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, BufferForAppendingHyphen* charactersWithHyphen) const
+TextRun InlineTextBox::constructTextRun(RenderStyle* style, const Font& font, const UChar* characters, int length, int maximumLength, BufferForAppendingHyphen* charactersWithHyphen) const
{
ASSERT(style);
RenderText* textRenderer = this->textRenderer();
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);
if (textRunNeedsRenderingContext(font))
run.setRenderingContext(SVGTextRunRenderingContext::create(textRenderer));
// 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;
}
Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (90900 => 90901)
--- trunk/Source/WebCore/rendering/InlineTextBox.h 2011-07-13 10:23:54 UTC (rev 90900)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h 2011-07-13 11:33:53 UTC (rev 90901)
@@ -100,7 +100,7 @@
LayoutUnit selectionHeight();
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:
virtual IntRect calculateBoundaries() const { return IntRect(x(), y(), width(), height()); }
