MantisBT - Zandronum
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0001212Zandronum[All Projects] Bugpublic2012-12-13 21:112018-09-30 20:46
ReporterDusk 
Assigned ToTorr Samaho 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version1.0 
Target Version1.1Fixed in Version1.1 
Summary0001212: Hold tight to your seats: Blood map reading messes up numsectors
DescriptionThis is quite a strange chain of events. Basically it seems that ZDoom can read Blood maps up to an extent, and what's up still remains functional. If someone tries to change to a map the engine deems worthy of checking whether's a Blood map, it does a series on checks on it (P_IsBuildMap) before loading it as such.

However, this series of checks involves changing numsectors with it assuming it's a Blood map... the line of code in question is p_buildmap.cpp:162. The line is also duplicated later on.

Here's the part which makes this a bit more scary: it seems that some lumps, ZDoom Wars' text file in particular, gets P_IsBuildMap'd. numsectors gets messed up and a crash is triggered by unlagged, which relies on numsectors in its sector building mode. This made the ZDoom Wars server on Grandvoid vulnerable. Two users found this and went as far to exploit it against another player, earning bans in the process...
Steps To Reproduce- Load up a server with attached zdwarstest.pk3. No clients needed.
- changemap zdoomwar
Additional InformationCommenting out p_buildmap.cpp:162 fixes the crash. Latest ZDoom does not seem to exhibit the crash.
TagsNo tags attached.
Relationships
Attached Files? zdwarstest.pk3 (13,377) 2012-12-13 21:11
/tracker/file_download.php?file_id=901&type=bug
Issue History
Date ModifiedUsernameFieldChange
2012-12-13 21:11DuskNew Issue
2012-12-13 21:11DuskFile Added: zdwarstest.pk3
2012-12-13 21:13DuskAdditional Information Updatedbug_revision_view_page.php?rev_id=3020#r3020
2012-12-13 21:15DuskDescription Updatedbug_revision_view_page.php?rev_id=3022#r3022
2012-12-27 12:34Torr SamahoNote Added: 0005590
2012-12-27 12:34Torr SamahoStatusnew => feedback
2012-12-27 13:08DuskNote Added: 0005591
2012-12-27 13:08DuskStatusfeedback => new
2013-01-02 17:02Torr SamahoNote Added: 0005662
2013-01-02 17:02Torr SamahoAssigned To => Torr Samaho
2013-01-02 17:02Torr SamahoStatusnew => needs testing
2013-01-02 17:02Torr SamahoNote Edited: 0005662bug_revision_view_page.php?bugnote_id=5662#r3104
2013-01-02 17:02Torr SamahoNote Revision Dropped: 5662: 0003103
2013-01-02 17:03Torr SamahoProduct Version => 1.0
2013-01-02 17:03Torr SamahoTarget Version => 1.1
2013-02-06 20:35DuskNote Added: 0005949
2013-02-06 20:35DuskStatusneeds testing => resolved
2013-02-06 20:35DuskFixed in Version => 1.1
2013-02-06 20:35DuskResolutionopen => fixed
2013-02-06 20:36DuskStatusresolved => feedback
2013-02-06 20:36DuskResolutionfixed => reopened
2013-02-06 20:36DuskStatusfeedback => resolved
2013-02-06 20:36DuskResolutionreopened => fixed
2013-02-06 20:36DuskView Statusprivate => public
2018-09-30 20:46Blzut3Statusresolved => closed

Notes
(0005590)
Torr Samaho   
2012-12-27 12:34   
Are you sure that zdwarstest.pk3 contains the map? I just downloaded it and it only seems to contain ZDoomWarsIv2.6.txt (a ZDoom Wars readme).
(0005591)
Dusk   
2012-12-27 13:08   
The map doesn't exist, the readme file gets truncated down to "ZDOOMWAR" when the file is parsed (like how textcolors.txt goes down to "TEXTCOLO"). However, when "changemap zdoomwar" is attempted, it tries to read the text file as a Build map, and that's when things go wonky.
(0005662)
Torr Samaho   
2013-01-02 17:02   
Ah, I see.'https://bitbucket.org/Torr_Samaho/zandronum/commits/f4a49c128b33ca263d043543efb9ef21c1001a2d [^]' should take care of the issue. That's a bug of ZDoom's P_IsBuildMap implementation and should also be fixed in ZDoom.
(0005949)
Dusk   
2013-02-06 20:35   
Since the given example wad is also the only thing that can possibly trigger this problem, all that can be tested on this bug is that the example wad works properly - which it does. So I'm marking this as fixed.