Merge pull request #611 from zzam/pathfinder

Pathfinder: Fix out-of-bounds read/write at load/store of games
Wargus · Feb 26, 2024 · 34ec4cb · 34ec4cb
2 parents 35df666 + a214447
commit 34ec4cb
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 11 deletions.
diff --git a/src/include/pathfinder.h b/src/include/pathfinder.h
@@ -83,7 +83,7 @@ class PathFinderInput
 	void SetMinRange(int range);
 	void SetMaxRange(int range);
 
-	void PathRacalculated();
+	void PathRecalculated();
 
 	void Save(CFile &file) const;
 	void Load(lua_State *l);

diff --git a/src/pathfinder/pathfinder.cpp b/src/pathfinder/pathfinder.cpp
@@ -368,7 +368,7 @@ void PathFinderInput::SetMaxRange(int range)
 	}
 }
 
-void PathFinderInput::PathRacalculated()
+void PathFinderInput::PathRecalculated()
 {
 	unitSize.x = unit->Type->TileWidth;
 	unitSize.y = unit->Type->TileHeight;
@@ -405,18 +405,23 @@ static int NewPath(PathFinderInput &input, PathFinderOutput &output)
 						  input.GetMinRange(), input.GetMaxRange(),
 						  path, PathFinderOutput::MAX_PATH_LENGTH,
 						  *input.GetUnit());
-	input.PathRacalculated();
+	input.PathRecalculated();
 	if (i == PF_FAILED) {
 		i = PF_UNREACHABLE;
 	}
 
 	// Update path if it was requested. Otherwise we may only want
 	// to know if there exists a path.
 	if (path != nullptr) {
-		output.Length = std::min<int>(i, PathFinderOutput::MAX_PATH_LENGTH);
-		output.OverflowLength = std::min<int>(i - output.Length, PathFinderOutput::MAX_OVERFLOW);
-		if (output.Length == 0) {
-			++output.Length;
+		if (i >= 0) {
+			output.Length = std::min<int>(i, PathFinderOutput::MAX_PATH_LENGTH);
+			output.OverflowLength = std::min<int>(i - output.Length, PathFinderOutput::MAX_OVERFLOW);
+			if (output.Length == 0) {
+				++output.Length;
+			}
+		} else {
+			output.Length = 0;
+			output.OverflowLength = 0;
 		}
 	}
 	return i;

diff --git a/src/unit/script_unit.cpp b/src/unit/script_unit.cpp
@@ -257,10 +257,13 @@ void PathFinderOutput::Load(lua_State *l)
 				LuaError(l, "incorrect argument _");
 			}
 			const int subargs = lua_rawlen(l, -1);
-			for (int k = 0; k < subargs; ++k) {
-				this->Path[k] = LuaToNumber(l, -1, k + 1);
+			if (subargs <= PathFinderOutput::MAX_PATH_LENGTH)
+			{
+				for (int k = 0; k < subargs; ++k) {
+					this->Path[k] = LuaToNumber(l, -1, k + 1);
+				}
+				this->Length = subargs;
 			}
-			this->Length = subargs;
 			lua_pop(l, 1);
 		} else {
 			LuaError(l, "PathFinderOutput::Load: Unsupported tag: %s", tag.data());

diff --git a/src/unit/unit_save.cpp b/src/unit/unit_save.cpp
@@ -101,7 +101,7 @@ void PathFinderOutput::Save(CFile &file) const
 	if (this->OverflowLength) {
 		file.printf("\"overflow-length\", %d, ", this->OverflowLength);
 	}
-	if (this->Length > 0) {
+	if (this->Length > 0 && this->Length <= PathFinderOutput::MAX_PATH_LENGTH) {
 		file.printf("\"path\", {");
 		for (int i = 0; i < this->Length; ++i) {
 			file.printf("%d, ", this->Path[i]);