Race condition in cursor move.

The method WordComposer.moveCursorByAndReturnIfInsideComposingWord() iterates
through all the code points in the word that's currently being composed, and
it tries to adjust the cursor position by a given amount (left or right).
It copies the code points to a new array while processing. But the code point
count comes from a member variable. If the member variable changes while the
method is processing the copy of the code points, it can run over the length
of the code point array.

Bug 18876474.

Change-Id: Ib3a2d90a4e82b76d381efa774e6b3d6bca99c869
This commit is contained in:
Dan Zivkovic 2015-02-27 10:13:06 -08:00
parent 8472b36886
commit 69c04cadc7

View file

@ -232,31 +232,33 @@ public final class WordComposer {
* @return true if the cursor is still inside the composing word, false otherwise.
*/
public boolean moveCursorByAndReturnIfInsideComposingWord(final int expectedMoveAmount) {
int actualMoveAmountWithinWord = 0;
int actualMoveAmount = 0;
int cursorPos = mCursorPositionWithinWord;
// TODO: Don't make that copy. We can do this directly from mTypedWordCache.
final int[] codePoints = StringUtils.toCodePointArray(mTypedWordCache);
if (expectedMoveAmount >= 0) {
// Moving the cursor forward for the expected amount or until the end of the word has
// been reached, whichever comes first.
while (actualMoveAmountWithinWord < expectedMoveAmount && cursorPos < mCodePointSize) {
actualMoveAmountWithinWord += Character.charCount(codePoints[cursorPos]);
while (actualMoveAmount < expectedMoveAmount && cursorPos < codePoints.length) {
actualMoveAmount += Character.charCount(codePoints[cursorPos]);
++cursorPos;
}
} else {
// Moving the cursor backward for the expected amount or until the start of the word
// has been reached, whichever comes first.
while (actualMoveAmountWithinWord > expectedMoveAmount && cursorPos > 0) {
while (actualMoveAmount > expectedMoveAmount && cursorPos > 0) {
--cursorPos;
actualMoveAmountWithinWord -= Character.charCount(codePoints[cursorPos]);
actualMoveAmount -= Character.charCount(codePoints[cursorPos]);
}
}
// If the actual and expected amounts differ, we crossed the start or the end of the word
// so the result would not be inside the composing word.
if (actualMoveAmountWithinWord != expectedMoveAmount) return false;
if (actualMoveAmount != expectedMoveAmount) {
return false;
}
mCursorPositionWithinWord = cursorPos;
mCombinerChain.applyProcessedEvent(mCombinerChain.processEvent(mEvents,
Event.createCursorMovedEvent(cursorPos)));
mCombinerChain.applyProcessedEvent(mCombinerChain.processEvent(
mEvents, Event.createCursorMovedEvent(cursorPos)));
return true;
}