search

stujones11 / minetest-3d_armor

Visible player armor & wielded items for minetest
Other
56 stars 98 forks source link

You can put anything in armor slots #141

Closed Elkien3 closed 6 years ago

Elkien3 commented 6 years ago

https://youtu.be/a_iIViSz7_8 you can stack more than one of any type of armor, or put in any other type of item using a "swap" trick.

stujones11 commented 6 years ago

This seems to be a regression introduced by 59b26b37f9c7b68cd5ebecfb2efece1a9535b5dc

The 'swap' trick has always been possible and actually appears to be a bug with detached inventories since the allow_put callback gets bypassed in this case. However, prior to that commit those items were never really 'equipped'.

I think the best solution for now would be to validate the item again in on_put and return it to the main inventory if not an armor item. This should not really be necessary if the allow_put functioned correctly but this whole mess will eventually be replaced by player inventory callbacks and meta storage for MT version 0.5.0.

Thank you for the report.

stujones11 commented 6 years ago

This turned out to be more difficult than I expected because not only allow_put but also on_put gets bypassed by this 'trick' and the detached inventory is silently altered.

The only option left was to purge the armor inventory immediately prior to saving 21b5c685051d5681de7a526ed7b30992556878b5 This might still have unexpected results when using the swap trick, however, it should no longer be possible to exploit the detached inventory bug or lose anything in the process.

Please let me know if this fixes the problem satisfactorily so I can update my other branches. Also I think that this, along with some other recent updates, probably warrants a version bump.

Minor optimization: e4b12558d48ea56a474210d96dd97181b1dee2c3

Elkien3 commented 6 years ago

seems good after quick check, if you want I can give you an update after a few weeks of use on my server

stujones11 commented 6 years ago

Thanks for letting me know, although I am not completely satisfied with this solution as there are still some issues regarding the api callbacks being incorrectly triggered. It is doubtful that many (if any) other mods have adopted those features yet but there could still be potential exploits if any were to.

stujones11 commented 6 years ago

83f3e01efab48fe46b6e00d41d31413a6ec728a6 should ensure that the api callbacks are now triggered correctly. It is a lot more work than should be necessary, however, I see no other way to prevent this exploit with MT 0.4.16 (stable)