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

Lava can burn, damage, and destroy entities client-side and lead to de-sync

    XMLWordPrintable

Details

    • Bug
    • Status: Resolved
    • Resolution: Fixed
    • 1.18.1
    • 22w07a
    • None
    • Plausible
    • Networking
    • Normal

    Description

      Code analysis using yarn mappings for 1.18:

      In Entity.java, within the baseTick method, there is this block of code that checks if the entity is in lava.

      if (this.world.isClient) {
          this.extinguish();
      } else if (this.fireTicks > 0) {
          // Fire tick logic omitted.
      }
      
      if (this.isInLava()) {
          this.setOnFireFromLava();
          this.fallDistance *= 0.5f;
      } 

       

      If so, then the entity is set on fire. Notice that the isInLava block is separate from the firetick block and is not wrapped by a check for isClient.
       

      public void setOnFireFromLava() {
          if (this.isFireImmune()) {
              return;
          }
          this.setOnFireFor(15);
          if (this.damage(DamageSource.LAVA, 4.0f)) {
              this.playSound(SoundEvents.ENTITY_GENERIC_BURN, 0.4f, 2.0f + this.random.nextFloat() * 0.4f);
          }
      }

      Within the setOnFireFromLava method, there is a call to damage the item with damage(). There is still no check for isClient.

      This method is implemented on a per entity type basis. Lets use ItemEntity as an example:

      public boolean damage(DamageSource source, float amount) {
      // ... omitted code.
          this.scheduleVelocityUpdate();
          this.health = (int)((float)this.health - amount);
          this.emitGameEvent(GameEvent.ENTITY_DAMAGED, source.getAttacker());
          if (this.health <= 0) {
              this.getStack().onItemEntityDestroyed(this); this.discard();
          }
          return true;
      }

      In this method, the item entity's health is decremented. If there is no health remaining, the entity is removed. Considering that there are no checks for isClient, this method can be called on the client just by item ticking in lava. In doing so, the client side is able to destroy  item entities independently from the server if the item is in lava.

      Consider a scenario where an item entity is in lava briefly, for example, a shulker box may be unloaded rapidly in this regard by destroying the box item entity with lava (a new feature in 1.17). The lava is removed quickly by a dispenser before the box's contents are burned. 

      If there is some delay in receiving the packets, the lava may be removed too late client-side. In such a scenario, the box content item entities can be destroyed by the client side alone – to the server, the item entities are fine. These ghost-items are a form of server-client de-sync. This is not good.

      A fix can be implemented by checking for isClient at any point in the process to damage the entity. The server should be fully authoritative when it comes to damage/entity deletion.

      Attachments

        Issue Links

          Activity

            People

              panda4994 [Mojang] Panda
              andrews54757 Andrew S
              Votes:
              10 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                CHK: