From 2f217dad5693fdd9ccdb934c7bbbf6f1302d2dfe Mon Sep 17 00:00:00 2001 From: Misa Date: Thu, 18 Jan 2024 20:47:09 -0800 Subject: [PATCH] Fix segfault: unwordwrap string w/ 2 start `\n`s This fixes a segmentation fault caused by an out-of-bounds indexing caused by an attempt to unwordwrap a string that starts with two newlines. The problem here is that in the branch of the function string_unwordwrap() where `consecutive_newlines == 1`, the function does not check that the string `result` isn't empty before attempting to index `result.size()-1`. If `result` is empty, then `result.size()` is 0, and `result.size()-1` becomes -1, and indexing a string at position -1 is always undefined behavior. Funnily enough, a similar indexing happens just a few lines down, but this time, there is a check to make sure that the string isn't empty first. I'm unsure of how Dav999 forgot that check a few lines earlier. This situation can happen in practice, with custom level localizations. I made a level with a filename of testloc.vvvvvv and created a file at lang/fr/levels/testloc/custom_cutscenes.xml with the following content: Then I switched to French, created a script named `test`, and created a text box that started with two newlines (so in total, the text box must be at least 3 lines in length). Running the script triggers the segfault when the text box is created. (Well, technically, on my machine, it triggers an assertion fail in libstdc++ and aborts, but that's basically the same thing.) To fix this while still preserving the exact amount of newlines, if `result` is empty, we add a newline instead of attempting to index the string. --- desktop_version/src/Font.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/desktop_version/src/Font.cpp b/desktop_version/src/Font.cpp index da2d2b65..38773453 100644 --- a/desktop_version/src/Font.cpp +++ b/desktop_version/src/Font.cpp @@ -1048,8 +1048,16 @@ std::string string_unwordwrap(const std::string& s) } else if (consecutive_newlines == 1) { - // The last character was already a newline, so change it back from the space we thought it should have become. - result[result.size()-1] = '\n'; + if (!result.empty()) + { + // The last character was already a newline, so change it back from the space we thought it should have become. + result[result.size()-1] = '\n'; + } + else + { + // The string starts with two or more newlines, in this case we didn't add the first one at all. + result.append("\n\n"); + } } consecutive_newlines++; }