From 25baa82d3d535657f6fc41fcb881801ba417e59b Mon Sep 17 00:00:00 2001
From: Jonathan G Rennison <j.g.rennison@gmail.com>
Date: Thu, 20 Jun 2024 00:09:19 +0100
Subject: [PATCH] Avoid nullptr reference undefined behaviour in saveload when
 discarding table string on load

---
 src/sl/order_sl.cpp |  4 ++--
 src/sl/saveload.cpp | 12 +++++++++---
 src/sl/saveload.h   |  2 +-
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/src/sl/order_sl.cpp b/src/sl/order_sl.cpp
index bb68a06a25a..f1ad9a8230a 100644
--- a/src/sl/order_sl.cpp
+++ b/src/sl/order_sl.cpp
@@ -325,7 +325,7 @@ static void SaveDispatchSchedule(DispatchSchedule &ds)
 		SlWriteUint32((uint32_t)names.size());
 		for (auto &it : names) {
 			SlWriteUint32(it.first);
-			SlStdString(it.second, SLE_STR);
+			SlStdString(&(it.second), SLE_STR);
 		}
 	}
 }
@@ -354,7 +354,7 @@ static void LoadDispatchSchedule(DispatchSchedule &ds)
 		btree::btree_map<uint32_t, std::string> &names = ds.GetSupplementaryNameMap();
 		for (uint32_t i = 0; i < string_count; i++) {
 			uint32_t key = SlReadUint32();
-			SlStdString(names[key], SLE_STR);
+			SlStdString(&(names[key]), SLE_STR);
 		}
 	}
 }
diff --git a/src/sl/saveload.cpp b/src/sl/saveload.cpp
index 1fe971db838..96ffef71d8c 100644
--- a/src/sl/saveload.cpp
+++ b/src/sl/saveload.cpp
@@ -1254,10 +1254,13 @@ static void SlString(void *ptr, size_t length, VarType conv)
  * @param ptr the string being manipulated
  * @param conv must be SLE_FILE_STRING
  */
-void SlStdString(std::string &str, VarType conv)
+void SlStdString(std::string *ptr, VarType conv)
 {
 	switch (_sl.action) {
 		case SLA_SAVE: {
+			dbg_assert(ptr != nullptr);
+			std::string &str = *ptr;
+
 			SlWriteArrayLength(str.size());
 			SlCopyBytes(str.data(), str.size());
 			break;
@@ -1270,6 +1273,9 @@ void SlStdString(std::string &str, VarType conv)
 				return;
 			}
 
+			dbg_assert(ptr != nullptr);
+			std::string &str = *ptr;
+
 			str.resize(len);
 			SlCopyBytes(str.data(), len);
 
@@ -1992,7 +1998,7 @@ bool SlObjectMemberGeneric(void *object, const SaveLoad &sld)
 					}
 					break;
 				}
-				case SL_STDSTR: SlStdString(*static_cast<std::string *>(ptr), sld.conv); break;
+				case SL_STDSTR: SlStdString(static_cast<std::string *>(ptr), sld.conv); break;
 				default: NOT_REACHED();
 			}
 			break;
@@ -2216,7 +2222,7 @@ std::vector<SaveLoad> SlTableHeader(const NamedSaveLoadTable &slt)
 				}
 
 				std::string key;
-				SlStdString(key, SLE_STR);
+				SlStdString(&key, SLE_STR);
 
 				auto sld_it = std::lower_bound(key_lookup.begin(), key_lookup.end(), key);
 				if (sld_it == key_lookup.end() || sld_it->name != key) {
diff --git a/src/sl/saveload.h b/src/sl/saveload.h
index 7939677dd55..10795c8be0a 100644
--- a/src/sl/saveload.h
+++ b/src/sl/saveload.h
@@ -1061,7 +1061,7 @@ void SlLoadFromBuffer(const uint8_t *buffer, size_t length, F proc)
 }
 
 void SlGlobList(const SaveLoadTable &slt);
-void SlStdString(std::string &str, VarType conv);
+void SlStdString(std::string *str, VarType conv);
 void SlArray(void *array, size_t length, VarType conv);
 void SlObject(void *object, const SaveLoadTable &slt);
 bool SlObjectMember(void *object, const SaveLoad &sld);