search

opentibiabr / canary

Canary Server 13.x for OpenTibia community.
https://docs.opentibiabr.com/
GNU General Public License v2.0
374 stars 617 forks source link

Item dupe by item manipulation right before server crash. #468

Closed un000000 closed 1 year ago

un000000 commented 2 years ago

Priority

Medium

Area

What happened?

It is possible to trick game into "thinking" that one item is stored in seveal places at once, essentially multiplying (duplicating or worse) the item. It requires at least two characters on two accounts, an item of any type in any quantity and a server crash to occur. Basically this is due to fact that canry database operations sequence do not satisfy Consistency/Isoaltion properties.

Steps to reproduce: 0) Prepare two characters, e.g. "Joe" and "Steve", and at least one item u want to copy, e.g. Ferumbras' Hat 1) Login on Joe and put Ferumbras' Hat into depot. 2) Logout and login on Joe. Also, from now, dont logout Joe. 3) Take Ferumbras' Hat from Joe depot into his hand 4) Login on Steve and move hat from Joe hand into Steve hand 5) Put Farumbras' Hat into Steve depot. 6) Logout and login Steve. Also, from now, dont logout Steve. 7) Cause a crash, or wait for random crash, if you cant reliably crash the server 8) a) If server save took place, go back to step 0 and repeat b) If crash did occur, wait for server to be up again, and login on both characters to discover that there is Ferumbras' Hat in Joe and Steve depot.

Here is a video of the process of above steps(ignore character names): variant normal server save and shutdown: https://streamable.com/y3qbdq variant server crash: https://streamable.com/c6ydgn

What OS are you seeing the problem on?

Windows

Code of Conduct

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 120 days with no activity.

gesior commented 1 year ago

There is one simple solution to this: make server that does not crash. Tibia.com took it to extreme level. They rollback server database to last server save after every crash.

Some people also use 'save on crash' code (gdb + some small changes in engine), to save players and houses after server crashed. Running code to save items after crash is not a good idea, as some item that made server crash can be saved in database, so I do not recommend it.

dudantas commented 1 year ago

There is one simple solution to this: make server that does not crash. Tibia.com took it to extreme level. They rollback server database to last server save after every crash.

Some people also use 'save on crash' code (gdb + some small changes in engine), to save players and houses after server crashed. Running code to save items after crash is not a good idea, as some item that made server crash can be saved in database, so I do not recommend it.

We totally agree with this. If you follow the pull request that was closed, you will see that there was a discussion on the subject. https://github.com/opentibiabr/canary/pull/469#discussion_r936897979

un000000 commented 1 year ago

We totally agree with this. If you follow the pull request that was closed, you will see that there was a discussion on the subject. #469 (comment)

TL;DR Currently if a person knows a way to crash server on demand he can ruin server economy in, i would say, about an hour. With my "fix" there is no more item copying on crash, but players cannot save their boss loot etc. with logout - they have to wait till next server save, which may never happen because of crash and as a result lose their valuable loot. In opinion of person reviewing the pull request, this tradeoff is too much. There are also other problems like other database connections using obsolete data, because there was no commit.

Better solution would be to finally optimize server save lag to miliseconds and save server every few seconds. I dont really have big knowledge on this, but i bet something like this could work: mirror server state in ram (20+ gB/s) synchronously, then asynchronously send this data using mirrored part to the message broker that will serialize it to database entities.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 120 days with no activity.

dudantas commented 1 year ago

Hello there! Thank you for reaching out to us and bringing up this issue. We do recognize the seriousness of this problem, but we have decided to prioritize our efforts on preventing server crashes and trying not to let more crashes be added (not intentional, but sometimes it happens). We understand that finding a permanent solution for this would require significant work and could have wide-ranging implications for the game's code and infrastructure. Nevertheless, we value your feedback and encourage you to keep providing suggestions and feedback to help us improve the game. If you have any further questions or concerns, please do not hesitate to contact us.