EssentialsX

Type of bug

Exploit

`/ess dump all` output

Don't have it (I'm just a user of a server)

Error log (if applicable)

No response

Bug description

If you make Signs to sell items and someone buys it with a full inventory, the sign will receive the money and loose the item while the buyer won't pay and doesn't get the item. -> Item gets deleted but money gets duplicated. This bug can be used to exploit whole servers using EssentialsX as a trading system.

Steps to reproduce

Place a sell sign.
Fill someone's inventory.
This person clicks on sell sign.

Expected behaviour

Do nothing

Actual behaviour

Delete item and dupe money

I am the admin of the server in question.

So here is the output of /ess dump all

I am unable to reproduce the behavior, but there is a small discrepancy in this.
Sell signs allow players to sell items to the server, and Buy signs allow players to buy items from the server. When using a sell sign, the player is never going to be paying any money, they are going to be paid for their items.

If you can reliably reproduce this using the latest versions of both Paper, and EssentialsX (Latest Dev build is fine), and only running EssentialsX, Official EssentialsX Addons, LuckPerms, and Vault; please show a video of this, it will help with replicating said behavior.
My experienced behavior following the listed criteria above is exactly as it should. if my inventory is full, i get the message saying my inventory is full

Confirmed, Thank you for the proper reproduction steps. I'll forward to the development team

@Evidentsinger14

@Evidentsinger14 it's not about buy/sell signs. It's about Trade Signs.

You need 2 players to reproduce the bug:

Bob places a Trade-Sign with those lines: (L1: [Trade], L2: <any price>$, L3: 1 shlkshell:1)
Alice fills her inventory to a point where she´s not able to pick up any shulker shell
Alice uses the Trade sign to buy shulker shells

Result:
Then the trade doesn´t actually happen, but the money in the sign increases.
Bob is now able to retreive money from the sign, which wasn´t actually spent by Alice.

This appears to be (at least partially) an API issue. I tested this on 1.18.2 and this was not an issue, but on 1.19.4 it is an issue.

The code below deals with the transaction, and rolls it back when the payment is not possible, for example, due to the player's inventory being full. This code appears to run as intended on both versions.

Essentials/Essentials/src/main/java/com/earth2me/essentials/signs/SignTrade.java

Lines 72 to 77 in 79449ef

    
           addAmount(sign, 1, charge, ess); 
        
           subtractAmount(sign, 2, trade, ess); 
        
           if (!trade.pay(player)) { 
        
               subtractAmount(sign, 1, charge, ess); 
        
               addAmount(sign, 2, trade, ess); 
        
               throw new ChargeException("Full inventory");

If you follow the logic for addAmount/removeAmount you eventually get to the setLine call here which actually updates the value on the sign.

Essentials/Essentials/src/main/java/com/earth2me/essentials/signs/SignTrade.java

Line 338 in 79449ef

sign.setLine(index, newline);

The problem is that when rolling back, the line is set back to the correct value, however this appears to get ignored when the sign actually updates.

Thanks, everyone!

	addAmount(sign, 1, charge, ess);
	subtractAmount(sign, 2, trade, ess);
	if (!trade.pay(player)) {
	subtractAmount(sign, 1, charge, ess);
	addAmount(sign, 2, trade, ess);
	throw new ChargeException("Full inventory");

Share to