Skip to content

Codex/maplelib review fixes #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lastbattle merged 3 commits into from
May 17, 2026
Merged

Codex/maplelib review fixes #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f89dd95
Fix IMG path traversal validation
lastbattle May 17, 2026
37ddb49
Handle empty WZ image paths
lastbattle May 17, 2026
d0e6a18
Require exact fixed-length string reads
lastbattle May 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 34 additions & 27 deletions MapleLib/Img/ImgFileSystemManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public ImgFileSystemManager(
if (!Directory.Exists(versionPath))
throw new DirectoryNotFoundException($"Version directory not found: {versionPath}");

_versionPath = versionPath;
_versionPath = Path.GetFullPath(versionPath);
_config = config ?? new HaCreatorConfig();

// Initialize LRU cache with size-based eviction
Expand All @@ -122,12 +122,12 @@ public ImgFileSystemManager(
_imageCache = new LRUCache<string, WzImage>(maxCacheBytes, EstimateWzImageSize);

// Load version manifest
string manifestPath = Path.Combine(versionPath, MANIFEST_FILENAME);
string manifestPath = Path.Combine(_versionPath, MANIFEST_FILENAME);
if (File.Exists(manifestPath))
{
string json = File.ReadAllText(manifestPath);
_versionInfo = Newtonsoft.Json.JsonConvert.DeserializeObject<VersionInfo>(json);
_versionInfo.DirectoryPath = versionPath;
_versionInfo.DirectoryPath = _versionPath;
}
else
{
Expand All @@ -136,7 +136,7 @@ public ImgFileSystemManager(
{
Version = Path.GetFileName(versionPath),
DisplayName = Path.GetFileName(versionPath),
DirectoryPath = versionPath,
DirectoryPath = _versionPath,
ExtractedDate = DateTime.Now
};
}
Expand Down Expand Up @@ -269,9 +269,7 @@ public WzImage LoadImage(string category, string relativePath)
Interlocked.Increment(ref _cacheMisses);

// Build full path
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
string fullPath = GetContainedImagePath(category, relativePath);

if (!File.Exists(fullPath))
return null;
Expand Down Expand Up @@ -489,20 +487,15 @@ public bool CategoryExists(string category)
/// </summary>
public bool ImageExists(string category, string relativePath)
{
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
return File.Exists(fullPath);
return File.Exists(GetContainedImagePath(category, relativePath));
}

/// <summary>
/// Gets diagnostic information about an image lookup for debugging purposes
/// </summary>
public string GetImageDiagnostics(string category, string relativePath)
{
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
string fullPath = GetContainedImagePath(category, relativePath);

string categoryPath = Path.Combine(_versionPath, category);
bool categoryDirExists = Directory.Exists(categoryPath);
Expand Down Expand Up @@ -557,9 +550,7 @@ public bool SaveImage(WzImage image, string category, string relativePath)
if (image == null)
throw new ArgumentNullException(nameof(image));

string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
string fullPath = GetContainedImagePath(category, relativePath);

return SaveImageToFile(image, fullPath);
}
Expand Down Expand Up @@ -619,6 +610,29 @@ public bool SaveImageToFile(WzImage image, string filePath)
}
}

private string GetContainedImagePath(string category, string relativePath)
{
if (string.IsNullOrWhiteSpace(category))
throw new ArgumentException("Category is required.", nameof(category));
if (string.IsNullOrWhiteSpace(relativePath))
throw new ArgumentException("Relative path is required.", nameof(relativePath));

string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";

fullPath = Path.GetFullPath(fullPath);
string root = Path.GetFullPath(_versionPath);
string rootWithSeparator = Path.EndsInDirectorySeparator(root)
? root
: root + Path.DirectorySeparatorChar;

if (!fullPath.StartsWith(rootWithSeparator, StringComparison.OrdinalIgnoreCase))
throw new InvalidOperationException($"Image path escapes the version directory: {fullPath}");

return fullPath;
}

/// <summary>
/// Gets the cache key from a full file path
/// </summary>
Expand All @@ -644,9 +658,7 @@ private string GetCacheKeyFromPath(string filePath)
/// <returns>The created WzImage or null if failed</returns>
public WzImage CreateImage(string category, string relativePath)
{
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
string fullPath = GetContainedImagePath(category, relativePath);

// Check if file already exists
if (File.Exists(fullPath))
Expand Down Expand Up @@ -689,9 +701,7 @@ public WzImage CreateImage(string category, string relativePath)
/// <returns>True if deleted successfully</returns>
public bool DeleteImage(string category, string relativePath)
{
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
string fullPath = GetContainedImagePath(category, relativePath);

try
{
Expand Down Expand Up @@ -724,10 +734,7 @@ public bool DeleteImage(string category, string relativePath)
/// </summary>
public string GetImagePath(string category, string relativePath)
{
string fullPath = Path.Combine(_versionPath, category, relativePath);
if (!fullPath.EndsWith(".img", StringComparison.OrdinalIgnoreCase))
fullPath += ".img";
return fullPath;
return GetContainedImagePath(category, relativePath);
}
#endregion

Expand Down
4 changes: 2 additions & 2 deletions MapleLib/WzLib/Util/WzBinaryReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private string DecodeUnicode(int length)
{
char[]? pooledArray = null;

Check warning on line 113 in MapleLib/WzLib/Util/WzBinaryReader.cs

View workflow job for this annotation

GitHub Actions / build 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
try
{
Span<char> chars = length <= MemoryLimits.STACKALLOC_SIZE_LIMIT_L1
Expand Down Expand Up @@ -148,7 +148,7 @@
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private string DecodeAscii(int length)
{
byte[]? pooledArray = null;

Check warning on line 151 in MapleLib/WzLib/Util/WzBinaryReader.cs

View workflow job for this annotation

GitHub Actions / build 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
try
{
Span<byte> bytes = length <= MemoryLimits.STACKALLOC_SIZE_LIMIT_L1
Expand Down Expand Up @@ -184,14 +184,14 @@
/// <param name="filePath">Length of bytes to read</param>
public string ReadString(int length)
{
byte[]? pooledArray = null;

Check warning on line 187 in MapleLib/WzLib/Util/WzBinaryReader.cs

View workflow job for this annotation

GitHub Actions / build 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
try
{
Span<byte> buffer = length <= MemoryLimits.STACKALLOC_SIZE_LIMIT_L1
? stackalloc byte[length]
: (pooledArray = s_bytePool.Rent(length)).AsSpan(0, length);

BaseStream.Read(buffer);
BaseStream.ReadExactly(buffer);
return Encoding.ASCII.GetString(buffer);
}
finally
Expand All @@ -206,7 +206,7 @@
public string ReadNullTerminatedString()
{
const int initialBufferSize = 256;
byte[]? pooledArray = null;

Check warning on line 209 in MapleLib/WzLib/Util/WzBinaryReader.cs

View workflow job for this annotation

GitHub Actions / build 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
try
{
Span<byte> buffer = stackalloc byte[initialBufferSize];
Expand Down Expand Up @@ -296,7 +296,7 @@
/// <returns></returns>
public string DecryptString(ReadOnlySpan<char> stringToDecrypt)
{
char[]? pooledArray = null;

Check warning on line 299 in MapleLib/WzLib/Util/WzBinaryReader.cs

View workflow job for this annotation

GitHub Actions / build 

The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.
try
{
Span<char> outputChars = stringToDecrypt.Length <= MemoryLimits.STACKALLOC_SIZE_LIMIT_L1
Expand Down Expand Up @@ -444,4 +444,4 @@
}
#endregion
}
}
}
6 changes: 6 additions & 0 deletions MapleLib/WzLib/WzImage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,14 @@ public WzImageProperty GetFromPath(string path)
{
if (reader != null) if (!parsed) ParseImage();

if (string.IsNullOrWhiteSpace(path))
return null;

string[] segments = path.Split(new char[1] { '/' }, System.StringSplitOptions.RemoveEmptyEntries);

if (segments.Length == 0)
return null;

// If the first segment is "..", return null
if (segments[0] == "..")
return null;
Expand Down
Loading