From 88324a253ecb5bc0b458d2c726e0696434cb291e Mon Sep 17 00:00:00 2001
From: Jonathan G Rennison <j.g.rennison@gmail.com>
Date: Mon, 1 Jan 2024 18:26:31 +0000
Subject: [PATCH] Fix #11644: Off by one error/buffer over-read in StrMakeValid
 (#11645)

* Fix #11644: Off by one error in StrMakeValid UTF-8 decode overrun detection

* Fix #11644: Off by one error in StrMakeValid buffer last character

* Fix: Unnecessary string duplication at StrMakeValid call sites
---
 src/fileio.cpp   | 2 +-
 src/ini_load.cpp | 2 +-
 src/string.cpp   | 6 ++++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/fileio.cpp b/src/fileio.cpp
index b0edd86b5c..215da0f36b 100644
--- a/src/fileio.cpp
+++ b/src/fileio.cpp
@@ -486,7 +486,7 @@ static std::string ExtractString(char *buffer, size_t buffer_length)
 {
 	size_t length = 0;
 	for (; length < buffer_length && buffer[length] != '\0'; length++) {}
-	return StrMakeValid(std::string(buffer, length));
+	return StrMakeValid(std::string_view(buffer, length));
 }
 
 bool TarScanner::AddFile(const std::string &filename, size_t, [[maybe_unused]] const std::string &tar_filename)
diff --git a/src/ini_load.cpp b/src/ini_load.cpp
index cc7d681467..1a6eed76f5 100644
--- a/src/ini_load.cpp
+++ b/src/ini_load.cpp
@@ -284,7 +284,7 @@ void IniLoadFile::LoadFromDisk(const std::string &filename, Subdirectory subdir)
 			if (!quoted && e == t) {
 				item.value.reset();
 			} else {
-				item.value = StrMakeValid(std::string(t));
+				item.value = StrMakeValid(std::string_view(t));
 			}
 		} else {
 			/* it's an orphan item */
diff --git a/src/string.cpp b/src/string.cpp
index 020fd2fd47..4eb45c2886 100644
--- a/src/string.cpp
+++ b/src/string.cpp
@@ -141,7 +141,7 @@ static void StrMakeValid(T &dst, const char *str, const char *last, StringValida
 		 * would also reach the "last" byte of the string and a normal '\0'
 		 * termination will be placed after it.
 		 */
-		if (len == 0 || str + len > last || len != Utf8Decode(&c, str)) {
+		if (len == 0 || str + len > last + 1 || len != Utf8Decode(&c, str)) {
 			/* Maybe the next byte is still a valid character? */
 			str++;
 			continue;
@@ -211,8 +211,10 @@ void StrMakeValidInPlace(char *str, StringValidationSettings settings)
  */
 std::string StrMakeValid(std::string_view str, StringValidationSettings settings)
 {
+	if (str.empty()) return {};
+
 	auto buf = str.data();
-	auto last = buf + str.size();
+	auto last = buf + str.size() - 1;
 
 	std::ostringstream dst;
 	std::ostreambuf_iterator<char> dst_iter(dst);