PneumaticCraft: Repressurized

PneumaticCraft: Repressurized

50M Downloads

Amadron Custom Trade bug

bottAy opened this issue ยท 7 comments

commented

Minecraft Version 1.16.4

Forge Version 35.1.36

Mod Version pneumaticcraft-repressurized-1.16.5-2.9.4-126 (direwolf pack)

Describe your problem, including steps to reproduce it I'm admining a server, and someone has found a way into custom amadron trades, to the point of buying items from himself that he doesn't have. I don't have the steps sadly, as it wasn't myself that discovered it, but Here's the line and the "error" that still let him around it,

"[03Feb2021 15:46:54.735] [Server thread/WARN] [pneumaticcraft/]: in-stock for [id = pneumaticcraft:PlayerName, in = 1 x Redstone Dust, out = 6 x Nether Star, level = 0, max = -1] - PlayerName dropped to -2? shouldn't happen!"

"[03Feb2021 16:09:55.336] [Server thread/WARN] [pneumaticcraft/]: in-stock for [id = pneumaticcraft:PlayerName, in = 1 x Redstone Dust, out = 2 x Totem of Undying, level = 0, max = -1] - PlayerName dropped to -2? shouldn't happen!"

"[03Feb2021 16:35:34.834] [Server thread/WARN] [pneumaticcraft/]: in-stock for [id = pneumaticcraft:PlayerName, in = 1 x Redstone Dust, out = 64 x Advanced Liquid Compressor, level = 0, max = -1] - PlayerName dropped to -2? shouldn't happen!"

Any other comments?

Again, sorry that I do not have the steps, I've remedied this short term by disabling custom trades. I know he did end up receiving these items, as I had to remove them from the world edit/inventory during a emergency downtime.

commented

Without steps to reproduce, this is going to be tricky to track down. I'll go through the code for the ordering process and make sure there is sufficient server-side verification of the order at all steps (if I had to guess, he somehow spoofed the order message, bypassing the Amadron GUI which normally prevents an order being sent if there isn't enough of the item in stock at the time).

Difficult to know if any fix is the right one without knowing what he did to exploit the problem, though...

commented

I have spotted & fixed one problem where the server needs to do some more validation. The other possible issue is a race condition where multiple orders are sent very close together - I have some more testing to do on that, though.

commented

Also fixed a potential race condition with stock levels; they were being updated on a successful trade, which can be too late if multiple players place the same order at the same time, or one player places an order several times in quick succession.

Now it will reduce stock levels as soon as the order is placed, and restore them if the order fails for any reason (e.g. player takes the payment out of the chest before the Amadrone can collect it).

commented

Sorry haven't had time to reply, I did some replication testing last night and I did see your race situation. I also did come across another occurrence, of what I believe he managed to do consistently, but I couldn't replicate it more than the one time it just happened. I'm going to continue to see if I can pin point that down tonight. Thanks for the quick work on this issue.

commented

2.10.0 beta release is out now and has the fixes I discussed in it. Would be interested to see if you can still find any exploits with that.

commented

Did you get a chance to do any testing on this?

commented

Closing due to lack of response. This should be fixed as far as I can tell, but feel free to reopen if you have any new info.