search

mbbsemu / MBBSEmu

The MajorBBS Emulation Project is an Open Source, Cross-Platform emulator for easily running The MajorBBS & Worldgroup Modules
https://www.mbbsemu.com
MIT License
134 stars 13 forks source link

[SFAZRG] Zorgon - Bad Parameters for PRFMSG (Invalid Pointer) #527

Closed enusbaum closed 1 year ago

enusbaum commented 2 years ago

Module Information

Describe the bug While running Zorgon (SFAZRG), you'll see multiple Invalid Pointer references while performing String formatting via PRFMSG or PRF. 

2022-01-02 21:10:45.9681 Info MBBSEmu.HostProcess.MbbsHost.ProcessRTKICK Running RTKICK-2: 0004:0785
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.setmbk (SFAZRG) Enqueue Previous MCV File: SFAZRG.MCV (Pointer: FFFF:0000)
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.setmbk (SFAZRG) Set Current MCV File: SFAZRG.MCV (Pointer: FFFF:0000)
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.tolower (SFAZRG) Converted c to c
2022-01-02 21:10:45.9681 Error MBBSEmu.HostProcess.ExportedModules.ExportedModuleBase.FormatPrintf (SFAZRG) Invalid Pointer: 1600:0000
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.prfmsg (SFAZRG) Added 60 bytes to the buffer from message number 531
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.clrprf (SFAZRG) Reset Output Buffer
2022-01-02 21:10:45.9681 Debug MBBSEmu.HostProcess.ExportedModules.Majorbbs.rtkick (SFAZRG) Registered routine 0004:0785 to execute every 5 seconds

image

Investigating this, it looks like there's a parameter being passed into PRFMSG that is invalid. From what I can tell from the disassembly, the call seems to be something like this:

prfmsg(500, subroutine1(arg), subroutine2(arg), subroutine3(arg));

The RTKICK log above, the final parameter is the invalid pointer. This will require a little more intricate stack debugging. I'm going to open this as it MIGHT be an issue that'll surface in another module which could be easier to debug.

paladine commented 2 years ago

This might actually be working as intended. The code checks ProtectedMemory.HasSegment to see if the segment is valid before trying to print the string, but the memory address shown here is a heap address, so it's not tracked by the _segments inside the ProtectedMemoryCore but rather tracked by the heap allocator.

We should probably just remove the safety check and let it try to dereference whatever address it gets.

paladine commented 2 years ago

Nevermind, I could be mistaken here. It seems we do track heap segments via an AddSegment call when we need a new segment for the heap, so HasSegment should work in that case. Plus the 0x1600 segment seems quite high, since 0x600 segments would have to have been allocated via the heap, which sounds pretty crazy

enusbaum commented 1 year ago

Verified this is fixed in https://github.com/mbbsemu/MBBSEmu/tree/v1.0-alpha-111223

image