Uploaded image for project: 'Minecraft: Java Edition'
  1. Minecraft: Java Edition
  2. MC-206338

Potential CME in skeleton trap code

XMLWordPrintable

    • Icon: Bug Bug
    • Resolution: Unresolved
    • None
    • 1.16.4, 20w46a
    • None
    • Plausible
    • Crash
    • Normal
    • Platform

      All code references are using official mappings for 20w46a.

      The skeleton trap code currently relies on the fact that

      1. the goal used for the skeleton traps is always the last in the LinkedHashSet used for GoalSelector.availableGoals and
      2. the (current) iterator implementation for LinkedHashSet only checks for concurrent modifications in next, but not in hasNext

      GoalSelector.tick calls getRunningGoals().forEach(WrappedGoal::tick);. SkeletonTrapGoal.tick calls SkeletonHorse.setTrap, which directly modifies GoalSelector.availableGoals, the underlying set for the stream returned by getRunningGoals(). This is generally bad style, and in this case relies on undocumented behavior of LinkedHashSet iterators (the second point above): As the last element in the set is the one modifying the set, Iterator.next is not called after the modification, only Iterator.hasNext is. If any goals are added to the set after the trap goal, triggering the trap will crash the server (this happened on a modded server, that's how I found the issue). This never happens in vanilla, but I would still consider it a vanilla bug as it relies on undocumented behavior that could change in any Java update.

      The cleanest fix for this would probably be to add a second boolean to SkeletonHorse indicating whether the horse should remain a trap, and to update the goals in SkeletonHorse.tick if this boolean does not match isTrap.

            Unassigned Unassigned
            malte0811 malte0811
            Votes:
            4 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              CHK: