[scintilla:bugs] #2513 Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

0 views
Skip to first unread message

Zufu Liu

unread,
Jul 31, 2026, 6:51:42 AMJul 31
to scintill...@googlegroups.com

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu
Last Updated: Fri Jul 31, 2026 10:51 AM UTC
Owner: Neil Hodgson

When sText and sMapped have different lengths (e.g. sText="a"; sMapped="aa"), the two while loop may have out of bounds read, second loop may overflow.

Sent from sourceforge.net because scintill...@googlegroups.com is subscribed to https://sourceforge.net/p/scintilla/bugs/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/scintilla/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

Zufu Liu

unread,
Jul 31, 2026, 7:26:53 AMJul 31
to scintill...@googlegroups.com
  • Comment:

Following is the patch I currently test (needs better function/variable names), it's produced with following steps.
Simplify the code by adding 1 for lastDifferenceText and lastDifferenceMapped, this changed the code to following:

size_t firstDifference = 0;
while (sMapped[firstDifference] == sText[firstDifference]) {
    firstDifference++;
}
size_t lastDifferenceText = sText.size();
size_t lastDifferenceMapped = sMapped.size();
while (sMapped[lastDifferenceMapped - 1] == sText[lastDifferenceText - 1]) {
    lastDifferenceText--;
    lastDifferenceMapped--;
}
const size_t endDifferenceText = sText.size() - lastDifferenceText;
const Sci::Position lengthChange = lastDifferenceMapped - firstDifference;
// pdoc->DeleteChars() and pdoc->InsertString()

Fix out of bounds in the while loop, using code similar to Document::TrimReplacement():

Range FindDifference(std::string_view view, std::string_view text) noexcept {
    Range range;
    while (!view.empty() && !text.empty() && view.front() == text.front()) {
        range.start++;
        text.remove_prefix(1);
        view.remove_prefix(1);
    }
    while (!view.empty() && !text.empty() && view.back() == text.back()) {
        range.end++;
        text.remove_suffix(1);
        view.remove_suffix(1);
    }
    return range;
}

const Range difference = FindDifference(sMapped, sText);
const size_t firstDifference = difference.start;
const size_t lastDifferenceText = sText.size() - difference.end;
const size_t lastDifferenceMapped = sMapped.size() - difference.end;
const size_t endDifferenceText = sText.size() - lastDifferenceText;
// pdoc->DeleteChars() and pdoc->InsertString()

Extract common expression and simplify the arithmetic, got following:

const auto [firstDifference, endDifferenceText] = FindDifference(sMapped, sText);
const Sci::Position lengthSame = firstDifference + endDifferenceText;
const Sci::Position insertPos = currentNoVS.Start().Position() + firstDifference;
pdoc->DeleteChars(insertPos, rangeBytes - lengthSame);
const Sci::Position lengthChange = sMapped.size() - lengthSame;
const Sci::Position lengthInserted = pdoc->InsertString(
    insertPos,
    sMapped.c_str() + firstDifference,
    lengthChange);

Not sure whether worth it, pdoc->InsertString() can be simplified by pass first view by reference (as in TrimReplacement()).

std::string_view target = sMapped;
const auto [firstDifference, endDifferenceText] = FindDifference(target, sText);
const Sci::Position lengthSame = firstDifference + endDifferenceText;
const Sci::Position insertPos = currentNoVS.Start().Position() + firstDifference;
pdoc->DeleteChars(insertPos, rangeBytes - lengthSame);
const Sci::Position lengthChange = sMapped.size() - lengthSame;
const Sci::Position lengthInserted = pdoc->InsertString(insertPos, target);

or following code:

std::string_view target = sMapped;
const auto [firstDifference, endDifferenceText] = FindDifference(target, sText);
const Sci::Position insertPos = currentNoVS.Start().Position() + firstDifference;
pdoc->DeleteChars(insertPos, rangeBytes - firstDifference - endDifferenceText);
const Sci::Position lengthInserted = pdoc->InsertString(insertPos, target);
const Sci::Position diffSizes = sMapped.size() - sText.size() + lengthInserted - target.length();

Zufu Liu

unread,
Jul 31, 2026, 7:48:19 AMJul 31
to scintill...@googlegroups.com

or following:

std::string_view target = sMapped;
const auto [firstDifference, endDifferenceText] = FindDifference(target, sText);
const Sci::Position lengthSame = firstDifference + endDifferenceText;
const Sci::Position insertPos = currentNoVS.Start().Position() + firstDifference;

pdoc->DeleteChars(insertPos, rangeBytes - lengthSame);
const Sci::Position lengthInserted = pdoc->InsertString(insertPos, target);
const Sci::Position diffSizes = lengthSame - sText.size() + lengthInserted;

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Fri Jul 31, 2026 11:26 AM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jul 31, 2026, 5:24:47 PMJul 31
to scintill...@googlegroups.com

simplified the code further as following, moved two views above the if condition to avoid decouple SSO string data and length twice.

std::string_view text = sText;
std::string_view mapped = sMapped;
if (mapped != text) {
    size_t firstDifference = 0;
    // similar to Document::TrimReplacement()
    while (!mapped.empty() && !text.empty() && mapped.front() == text.front()) {
        firstDifference++;
        text.remove_prefix(1);
        mapped.remove_prefix(1);
    }
    while (!mapped.empty() && !text.empty() && mapped.back() == text.back()) {
        text.remove_suffix(1);
        mapped.remove_suffix(1);

    }
    const Sci::Position insertPos = currentNoVS.Start().Position() + firstDifference;

    pdoc->DeleteChars(insertPos, text.length());
    const Sci::Position lengthInserted = pdoc->InsertString(insertPos, mapped);
    // Automatic movement changes selection so reset to exactly the same as it was.
    const Sci::Position diffSizes = lengthInserted - text.length();
}

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Fri Jul 31, 2026 11:48 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jul 31, 2026, 7:58:14 PMJul 31
to scintill...@googlegroups.com

I'm not sure this is possible. There has to be an input text that case maps to a different output where one is smaller but the smaller is a suffix of the larger. "a" -> "aa" (or reverse) isn't a feasible case mapping.

For input > output, there has to be some prefix of the input that is removed by the case map function. For output > input, some new text has to be inserted at the front of the input. This sort of result could occur for a normalization function that, for example, removed combining accents but I don't think it will occur for case mapping.

There could be some point in hardening against potential problems, either bugs or limitations in any platform calls, with the CaseMapString method.

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Fri Jul 31, 2026 09:24 PM UTC
Owner: Neil Hodgson

Zufu Liu

unread,
Jul 31, 2026, 8:28:39 PMJul 31
to scintill...@googlegroups.com

That's not possible in sensible implementation, but new code is more simple and readable.

I patched ScintillaWin::CaseMapString() to do custom text transliteration (which is the easiest way to handle multiple selections, much easy than iterate and manipulate selections in application). I found out of bounds read when doing C escape/unescape (e.g. \\ => \\\\).

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Fri Jul 31, 2026 11:58 PM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Jul 31, 2026, 9:09:46 PMJul 31
to scintill...@googlegroups.com

OK, I'll look at this after the release.

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Sat Aug 01, 2026 12:28 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Aug 9, 2026, 2:01:44 AM (10 days ago) Aug 9
to scintill...@googlegroups.com

Committed with [92a455].

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open
Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Sat Aug 01, 2026 01:09 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Aug 9, 2026, 2:01:56 AM (10 days ago) Aug 9
to scintill...@googlegroups.com
  • status: open --> open-fixed

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: open-fixed


Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Sun Aug 09, 2026 06:01 AM UTC
Owner: Neil Hodgson

Neil Hodgson

unread,
Aug 12, 2026, 8:16:48 AM (7 days ago) Aug 12
to scintill...@googlegroups.com
  • status: open-fixed --> closed-fixed

[bugs:#2513] Potential out of bounds read inside `Editor::ChangeCaseOfSelection()`

Status: closed-fixed


Group: Bug
Labels: Scintilla selection replace
Created: Fri Jul 31, 2026 10:51 AM UTC by Zufu Liu

Last Updated: Sun Aug 09, 2026 06:01 AM UTC
Owner: Neil Hodgson

Reply all
Reply to author
Forward
0 new messages