Skip to content

Commit c4c1464

Browse files
committed
fix(security): VFS write overflow, file close robustness, catalog safety
VFS_WriteFile: - Add overflow check before rounding endPos up to 4KB boundary. If endPos > 0xFFFFF000, adding 4095 wraps to a small value, causing undersized allocation and subsequent buffer overflow on memcpy. VFS_CloseFile: - Remove restriction that only 'created' overlay entries can persist data (any overlay entry with file data should be saved) - NULL-out fileData/fileDataSize before freeing to prevent dangling pointer if NewPtr fails - Only update modTime if GetDateTime returns non-zero (avoid setting file timestamp to Mac epoch on failure) HFS_CatalogLookup: - Use strlen-based length comparison before character loop instead of relying on null terminator probe at entries[i].name[len]. While the original was safe (len <= 31, name[32]), the new approach is clearer and avoids assumptions about null termination of catalog names.
1 parent 03ea572 commit c4c1464

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

src/FS/hfs_catalog.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ bool HFS_CatalogLookup(HFS_Catalog* cat, DirID parentID, const char* name,
241241

242242
/* Find matching name (case-insensitive) */
243243
for (int i = 0; i < count; i++) {
244+
/* Verify entry name length matches before comparing */
245+
size_t entryLen = strlen(entries[i].name);
246+
if (entryLen != len) continue;
247+
244248
bool match = true;
245249
for (size_t j = 0; j < len; j++) {
246250
char c1 = entries[i].name[j];
@@ -252,7 +256,7 @@ bool HFS_CatalogLookup(HFS_Catalog* cat, DirID parentID, const char* name,
252256
break;
253257
}
254258
}
255-
if (match && entries[i].name[len] == '\0') {
259+
if (match) {
256260
*entry = entries[i];
257261
return true;
258262
}

src/FS/vfs.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -896,10 +896,12 @@ void VFS_CloseFile(VFSFile* file) {
896896
VFSVolume* vol = VFS_FindVolume(file->vref);
897897
if (vol) {
898898
VFSOverlayEntry* oe = VFS_FindOverlay(vol, file->fileID);
899-
if (oe && oe->created) {
899+
if (oe) {
900900
/* Free old persisted data */
901901
if (oe->fileData) {
902902
DisposePtr((Ptr)oe->fileData);
903+
oe->fileData = NULL;
904+
oe->fileDataSize = 0;
903905
}
904906
/* Copy current buffer to overlay */
905907
oe->fileData = (uint8_t*)NewPtr(file->memSize);
@@ -911,7 +913,9 @@ void VFS_CloseFile(VFSFile* file) {
911913
extern void GetDateTime(uint32_t* secs);
912914
uint32_t now = 0;
913915
GetDateTime(&now);
914-
oe->entry.modTime = now;
916+
if (now != 0) {
917+
oe->entry.modTime = now;
918+
}
915919
}
916920
}
917921
}
@@ -955,7 +959,8 @@ bool VFS_WriteFile(VFSFile* file, const void* buffer, uint32_t length, uint32_t*
955959
uint32_t endPos = file->memPosition + length;
956960

957961
if (endPos > file->memCapacity) {
958-
/* Grow buffer — round up to 4KB blocks */
962+
/* Grow buffer — round up to 4KB blocks (check for overflow in rounding) */
963+
if (endPos > (uint32_t)0xFFFFF000) return false; /* Would overflow when rounding up */
959964
uint32_t newCap = (endPos + 4095) & ~4095u;
960965
uint8_t* newBuf = (uint8_t*)NewPtr(newCap);
961966
if (!newBuf) return false;

0 commit comments

Comments
 (0)