Gittes-archive/new-horizons
2
0
Fork 0
mirror of https://github.com/Outer-Wilds-New-Horizons/new-horizons.git synced 2025-12-11 12:05:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add a security check to ScatterBuilder (#1109)

Browse Source 
## Security check
What currently happens is that if `preventOverlap` is not true then
there is no array of point emptied progressively and points are rolled
in a loop of size count for each prop. The problem is that the loop can
be infinite due to `i--; // Try this point again`. A reroll happens when
`height < propInfo.minHeight` or `height > propInfo.maxHeight`, height
being rolled between `heightMap.minHeight` and `heightMap.maxHeight`
(which are not the constraints above).
- This PR checks before the loop if there is less than 0.1% chance for
the upcoming heightMap rolls to be considered correct, and if not it
always skips this check, ignoring `propInfo.minHeight` and `.maxHeight`
to prevent a very long (possibly infinite) loop.
This commit is contained in:
Noah Pilarski 2025-08-04 19:03:44 -04:00 committed by GitHub
commit ced49c6606
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 13 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
NewHorizons/Builder/Props/ScatterBuilder.cs
View File

@ -80,6 +80,18 @@ namespace NewHorizons.Builder.Props
};
var scatterPrefab = DetailBuilder.Make(go, sector, mod, prefab, detailInfo);
bool reasonableHeightConstraints = true;
if (!propInfo.preventOverlap && (heightMapTexture != null) && (propInfo.minHeight != null || propInfo.maxHeight != null)) // If caution is relevant
{
var maxHeight = (propInfo.maxHeight != null ? Math.Min(propInfo.maxHeight, heightMap.maxHeight) : heightMap.maxHeight);
var minHeight = (propInfo.minHeight != null ? Math.Max(propInfo.minHeight, heightMap.minHeight) : heightMap.minHeight);
if ((maxHeight - minHeight) / (heightMap.maxHeight - heightMap.minHeight) < 0.001) // If height roll has less than 0.1% chance of being valid
{
NHLogger.LogError($"Ignoring minHeight/maxHeight for scatter of [{scatterPrefab.name}] to prevent infinite rerolls from too much constraint on height.");
reasonableHeightConstraints = false; // Ignore propInfo.min/maxHeight to prevent infinite rerolls
}
// That way, even if often not valid, it still won't loop much more than propInfo.count * 1000 per prop
}
for (int i = 0; i < propInfo.count; i++)
{
Vector3 point;
@ -113,7 +125,7 @@ namespace NewHorizons.Builder.Props
float relativeHeight = heightMapTexture.GetPixel((int)sampleX, (int)sampleY).r;
height = (relativeHeight * (heightMap.maxHeight - heightMap.minHeight) + heightMap.minHeight);
if ((propInfo.minHeight != null && height < propInfo.minHeight) || (propInfo.maxHeight != null && height > propInfo.maxHeight))
if (reasonableHeightConstraints && ((propInfo.minHeight != null && height < propInfo.minHeight) || (propInfo.maxHeight != null && height > propInfo.maxHeight)))
{
// Try this point again
i--;