Skip to content

Buffer overread in yax_updown via G_MoveMisc

moneytst.map

Reproduced with the attached map while using v1.5's stock CON files. Sanitizer-enabled build should detect it. Explanation:

  1. The map has multiple MONEY sprites, operated via G_MoveMisc.
  2. A subset of these will get below floor Z level, hence they'll be added to the sprite delete queue via A_AddToDeleteQueue and their picnums will be incremented to MONEY+1.
  3. Once the latter occurs while the queue is full, the first queued sprite will be deleted.
  4. Problem is, the main loop of G_MoveMisc will still process a deleted MONEY sprite as if it weren't deleted. In other words, it'll still be in the spritestat linked list for statnum STAT_MISC. Due to having the incremented picnum of MONEY+1, yax_getflorzofslope will be called, which in turn will call yax_getneighborsect.
  5. The last function will inspect yax_updown[sectnum][!cf]. However, since the sprite should be deleted at this point, sectnum will be set to MAXSECTORS, hence leading to the overread in question.

Similar errors might not be reproduced in other situations due to the usual sectors array having the room for MAXSECTORS + M32_FIXME_SECTORS sectors in total. The latter seemed to be introduced in: 4b576fcd