[Upstream] Fix out of bounds access in dtMergeCorridorStartMoved and …

…add tests

size can become negative if req > maxPath. This may happen when visited buffer is larger than path buffer.
Add tests to cover different use cases of the function including Should add visited points not present in path up to the path capacity to cover the fix.
List tests files explicitly. When new file is added CMake does not add it to the already generated list if GLOB is used.

- recastnavigation/recastnavigation@599fd0f
- recastnavigation/recastnavigation#635

replace comment in DtPathCorridor

checking

tt

npath

src/DotRecast.Detour/DtPathUtils.cs

@@ -181,6 +181,7 @@ public static int MergeCorridorStartMoved(ref List<long> path, int npath, int ma
 
             // Adjust beginning of the buffer to include the visited.
             List<long> result = new List<long>();
+
             // Store visited
             for (int i = nvisited - 1; i > furthestVisited; --i)
             {

test/DotRecast.Detour.Crowd.Test/DtPathCorridorTest.cs

@@ -17,25 +17,155 @@ misrepresented as being the original software.
 3. This notice may not be removed or altered from any source distribution.
 */
 
+using System;
 using System.Collections.Generic;
 using DotRecast.Core.Numerics;
-
 using Moq;
 using NUnit.Framework;
 
 namespace DotRecast.Detour.Crowd.Test;
 
-
 public class DtPathCorridorTest
 {
-    private readonly DtPathCorridor corridor = new DtPathCorridor();
-    private readonly IDtQueryFilter filter = new DtQueryDefaultFilter();
+    private DtPathCorridor corridor;
+    private IDtQueryFilter filter;
 
     [SetUp]
     public void SetUp()
     {
-        corridor.Init(256);
+        corridor = new DtPathCorridor();
+        corridor.Init(DtCrowdConst.MAX_PATH_RESULT);
         corridor.Reset(0, new RcVec3f(10, 20, 30));
+
+        filter = new DtQueryDefaultFilter();
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldHandleEmptyInput()
+    {
+        var path = new List<long>();
+        const int npath = 0;
+        const int maxPath = 0;
+        Span<long> visited = stackalloc long[0];
+        const int nvisited = 0;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(0));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldHandleEmptyVisited()
+    {
+        var path = new List<long> { 1 };
+        const int npath = 1;
+        const int maxPath = 1;
+        Span<long> visited = stackalloc long[0];
+        const int nvisited = 0;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(1));
+
+        var expectedPath = new List<long> { 1 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldHandleEmptyPath()
+    {
+        var path = new List<long>();
+        const int npath = 0;
+        const int maxPath = 0;
+        Span<long> visited = stackalloc long[] { 1 };
+        const int nvisited = 1;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(0));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldStripVisitedPointsFromPathExceptLast()
+    {
+        var path = new List<long> { 1, 2 };
+        const int npath = 2;
+        const int maxPath = 2;
+        Span<long> visited = stackalloc long[] { 1, 2 };
+        const int nvisited = 2;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(1));
+
+        var expectedPath = new List<long> { 2, 2 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldAddVisitedPointsNotPresentInPathInReverseOrder()
+    {
+        var path = new List<long> { 1, 2, 0 };
+        const int npath = 2;
+        const int maxPath = 3;
+        Span<long> visited = stackalloc long[] { 1, 2, 3, 4 };
+        const int nvisited = 3;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(3));
+
+        var expectedPath = new List<long> { 4, 3, 2 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldAddVisitedPointsNotPresentInPathUpToThePathCapacity()
+    {
+        var path = new List<long>() { 1, 2, 0 };
+        const int npath = 2;
+        const int maxPath = 3;
+        Span<long> visited = stackalloc long[] { 1, 2, 3, 4, 5 };
+        const int nvisited = 5;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(3));
+
+        var expectedPath = new List<long> { 5, 4, 3 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldNotChangePathIfThereIsNoIntersectionWithVisited()
+    {
+        var path = new List<long>() { 1, 2 };
+        const int npath = 2;
+        const int maxPath = 2;
+        Span<long> visited = stackalloc long[] { 3, 4 };
+        const int nvisited = 2;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(2));
+
+        var expectedPath = new List<long> { 1, 2 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldSaveUnvisitedPathPoints()
+    {
+        var path = new List<long>() { 1, 2, 0 };
+        const int npath = 2;
+        const int maxPath = 3;
+        Span<long> visited = stackalloc long[] { 1, 3 };
+        const int nvisited = 2;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(3));
+        var expectedPath = new List<long> { 3, 1, 2 };
+        Assert.That(path, Is.EqualTo(expectedPath));
+    }
+
+    [Test(Description = "dtMergeCorridorStartMoved")]
+    public void ShouldSaveUnvisitedPathPointsUpToThePathCapacity()
+    {
+        var path = new List<long>() { 1, 2 };
+        const int npath = 2;
+        const int maxPath = 2;
+        Span<long> visited = stackalloc long[] { 1, 3 };
+        const int nvisited = 2;
+        var result = DtPathUtils.MergeCorridorStartMoved(ref path, npath, maxPath, visited, nvisited);
+        Assert.That(result, Is.EqualTo(2));
+
+        var expectedPath = new List<long> { 3, 1 };
+        Assert.That(path, Is.EqualTo(expectedPath));
     }
 
     [Test]
@@ -56,7 +186,7 @@ public void ShouldKeepOriginalPathInFindCornersWhenNothingCanBePruned()
                 It.IsAny<int>(),
                 It.IsAny<int>())
             )
-            .Callback((RcVec3f startPos, RcVec3f endPos, List<long> path, int pathSize,
+            .Callback((RcVec3f startPos, RcVec3f endPos, List<long> path, int npath,
                 ref List<DtStraightPath> refStraightPath, int maxStraightPath, int options) =>
             {
                 refStraightPath = straightPath;
@@ -88,7 +218,7 @@ public void ShouldPrunePathInFindCorners()
                 ref It.Ref<List<DtStraightPath>>.IsAny,
                 It.IsAny<int>(),
                 It.IsAny<int>())
-            ).Callback((RcVec3f startPos, RcVec3f endPos, List<long> path, int pathSize,
+            ).Callback((RcVec3f startPos, RcVec3f endPos, List<long> path, int npath,
                 ref List<DtStraightPath> refStraightPath, int maxStraightPath, int options) =>
             {
                 refStraightPath = straightPath;