Avoid nullptr reference undefined behaviour in saveload when discardi…

…ng table string on load
JGRennison · Jun 19, 2024 · 805e976 · 805e976
1 parent cf7efc5
commit 805e976
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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);