Money Bank Dupe Bug Fix [Free + Source]

Users who are viewing this thread

domipoppe

Knight
Old Guard
People are able to dupe money on all script-sets at banks while dropping a money bag and using the bank the same time.
The issue on that is that they use macro software so that stuff happens in milliseconds and it's impossible to prevent.

So I decided to release my own patch.
It basically forbids people to drop money bags near a bank.
Go to module_scripts.py and look for (eq, ":event_type", client_event_drop_money_bag),

Make sure it looks like this:
Code:
      (else_try), # client requesting to drop a money bag
        (eq, ":event_type", client_event_drop_money_bag),
        (try_begin),
          (neq, "$g_game_type", "mt_no_money"),
          (store_script_param, ":gold_amount", 3),
          (try_begin), # for positive amounts, drop a money bag
            (gt, ":gold_amount", 0),
			(player_get_agent_id, ":player_agent_id", ":sender_player_id"),
			(agent_get_position, pos20, ":player_agent_id"),
			(position_set_z, pos20, 0),
			(assign, ":too_near", 0),
			(try_for_prop_instances, ":cur_bank", "spr_pw_item_chest_invisible"),
				(prop_instance_get_position, pos21, ":cur_bank"),
				(position_set_z, pos21, 0),
				(get_distance_between_positions_in_meters, ":cur_distance", pos20, pos21),
				(try_begin),
					(le, ":cur_distance", 8),
					(assign, ":too_near", 1),
				(try_end),
			(try_end),
			(try_begin),
				(eq, ":too_near", 1),
				(str_store_string, s58, "@You are too near of a bank!"),
				(multiplayer_send_string_to_player, ":sender_player_id", server_event_local_chat, s58),
			(else_try),
				(call_script, "script_cf_drop_money_bag_item", ":sender_player_id", ":gold_amount"),
			(try_end),
          (else_try), # for negative amounts, check the admin has permission then spawn them the absolute money amount
            (lt, ":gold_amount", 0),
            (player_is_admin, ":sender_player_id"),
            (player_slot_eq, ":sender_player_id", slot_player_admin_no_gold, 0),
            (val_mul, ":gold_amount", -1),
            (call_script, "script_player_adjust_gold", ":sender_player_id", ":gold_amount", 1),
            (assign, reg0, ":gold_amount"),
            (str_store_string, s3, "str_log_admin_cheat_money"),
            (str_store_player_username, s0, ":sender_player_id"),
            (player_get_unique_id, reg0, ":sender_player_id"),
            (server_add_message_to_log, "str_log_admin_target_self"),
          (try_end),
        (try_end),
      (else_try), # requesting a spawn point or switching spectator status

If you do that, the bug is fixed. If you use this scripts to fix the bug include me in your credits.
 
hi,im a asian server host ,i host a PWserver for 4 years,now i wannna add some functions to my code(IG commonds and hungery debuff system),can u help me to modify my code?i can pay for u
 
Thanks Illuminati for making this dupe public.


I'm not sure if this was present on Phoenix as I've never tried and never caught anyone doing it. However, we have developed a new banking UI into our PK module to allow the bug to be resolved without making it hard to deposit exact amount into banks.


As a side note, you have a typo in your error message displayed to the user. You may wish to fix it.
 
TommyBristol said:
Thanks Illuminati for making this dupe public.


I'm not sure if this was present on Phoenix as I've never tried and never caught anyone doing it. However, we have developed a new banking UI into our PK module to allow the bug to be resolved without making it hard to deposit exact amount into banks.


As a side note, you have a typo in your error message displayed to the user. You may wish to fix it.

No problem.

It's just the main issue of Warband and it's single thread message URL system. It can be abused in many ways.
Also, a big issue is that Warband itself doesn't check for if the client & server files are identic or not.

And, if you release PK as OSP as it is claimed it will be, you have one issue:

Scripters can locally modify the module (same for PW) and make such glitches easily work again.
Like call server messages clientside etc. This stuff needs to be secured and whilst releasing at as OSP it is hard to secure cause everyone can read the full code.
You see it by the NW troll bettygta etc.
 
domipoppe said:
TommyBristol said:
Thanks Illuminati for making this dupe public.


I'm not sure if this was present on Phoenix as I've never tried and never caught anyone doing it. However, we have developed a new banking UI into our PK module to allow the bug to be resolved without making it hard to deposit exact amount into banks.


As a side note, you have a typo in your error message displayed to the user. You may wish to fix it.

No problem.

It's just the main issue of Warband and it's single thread message URL system. It can be abused in many ways.
Also, a big issue is that Warband itself doesn't check for if the client & server files are identic or not.

And, if you release PK as OSP as it is claimed it will be, you have one issue:

Scripters can locally modify the module (same for PW) and make such glitches easily work again.
Like call server messages clientside etc. This stuff needs to be secured and whilst releasing at as OSP it is hard to secure cause everyone can read the full code.
You see it by the NW troll bettygta etc.


Yeah I think we just have to accept that M&B mods are gonna be unsecure to an extent. I don't really think closed source solves much either considering it's very easy to decompile people's modules using some tools on Github. Plus, I found some issues with M&B itself[/size] during development of PK that allow players to bypass things that should really be set purely by the server. I won't go into detail as I obviously don't really want people to abuse it, but it is rather annoying. Hopefully this won't be the case in Bannerlord.
 
domipoppe said:
Scripters can locally modify the module (same for PW) and make such glitches easily work again.
Like call server messages clientside etc. This stuff needs to be secured and whilst releasing at as OSP it is hard to secure cause everyone can read the full code.
You see it by the NW troll bettygta etc.
You can make some glitches work by modifying client side code (generally things like wall hacks or unlimited distance name labels, which are just revealing things the client knows but hides from the player), but most should be prevented in server code: messages received by the server from a client should be mistrusted wherever possible. For example, the standard PW code for transferring money from a castle money chest has the client side check for the distance to the chest, whether there is enough money in the chest, then sends the player id, chest instance id, and money to transfer; the server side receives those values and does all the checks again and more, so the distance to the chest is checked again, the money in the chest, also whether the player is in the right faction, has keys, or the chest is broken, and then only if all the checks pass on the server is the money actually transferred and the clients updated. You might notice that the same script cf_use_castle_money_chest is run on both the clients and the server, with some code being run on both, and other bits split up: I designed various scripts this way to share code, and they should be labeled at the first line comment whether server, client, or both.

The general principle is that script checks on the client side can be useful to run preparatory code to find things - which can be computationally expensive, avoiding load on the server where possible - and (on clients not modified to remove checks) to prevent idiots spamming buttons to flood the server with impossible request messages; then every client check before requesting something is also rechecked on the server if at all possible. On the server it is not necessary to check again for every cart that is in range of the player to attach, but the nearest cart that was found on the client side and requested to attach with a network message must be within range and pass every other check in server side code before attaching. This way it doesn't generally matter if the module system is public, since players can't affect the important script checks running on the server. Another example is the PW animation scripts on the server are checked to prevent players calling them too often and being annoying, with the server enforcing a cool down period; as opposed to (if I recall correctly, at least in early days) the animation scripts for Viking Conquest multiplayer, which were not even checked whether the right agent id was sent, so people with modified clients could force other player characters to stop and perform a specific animation using malicious request messages.

My guess is that the exploit of dropping money bags with various (non official) money banking scripts is caused by not removing the money from the player on the warband server before sending the transfer request to the external web server and database: if the player tries to deposit money, the gold amount deposited should be checked and then removed immediately, probably storing the subtracted gold amount in a player slot (something like slot_player_unconfirmed_gold_amount_deposited, also a slot_player_time_gold_deposited with the current mission time), then sending the deposit message to the web server, and if the web server replies quickly enough with a successful deposit message, the player slots are cleared and the money remains unmodified, otherwise the server checks players every now and then with unconfirmed deposits a minute or so old, and refunds the undeposited money value to the player with an error message, so player gold isn't lost due to web server disconnections. Maybe if the server receives a deposit confirmation from the web server for a player without the unconfirmed slots set, the money should be subtracted from the player (in case the web server finally confirmed a deposit after the server gave up and refunded). The player should be prevented from making any further deposits or withdrawals while awating confirmation; alternatively you could only subtract the player gold after a web server transfer was confirmed, but try to make all scripts that deal with player gold fail if the player is marked as awaiting a bank transfer confirmation, so players couldn't drop money bags or buy anything until the bank transfer was confirmed to go through (or you could try overdrafts with penalty interest, jail time, or whatever other things real banks do).

Any time a check is run then a message is sent off to make a modification based on that check in another process later on, race conditions could happen; so ideally the checks should happen immediately in the M&B script before the modification is completed, which since the module system interpreter is not multi threaded - to my knowledge - should prevent that type of exploit.

Server messages or scripts run client side on a modified client should not affect the server, just desynchronize what that player sees compared with the server and everyone else: the section for handling events from the server will not run on a server, since it is inside a block starting with neg|multiplayer_is_server; so it should not affect anything if a modified client sends something like server_event_agent_equip_armor. I don't remember testing whether client scripts can run multiplayer_send_x_to_player operations to other clients, but it seems unlikely, and that would just mess up what the other player sees, not the gold, items, combat calculations, etc. stored on the server.

If you are especially paranoid, it would be possible but annoying to maintain two code bases, with one module system that includes all code and is only run on the server, and a different but compatible module system with all server scripts removed, which is built and released to clients: the released module can then only be used to connect to servers, not host another server or check the server scripts for exploits. I thought about it for PW but decided it wasn't worth the bother, since I wanted to allow anybody in the world to host a server, and to eventually release all the code publicly anyway.
 
Vornne said:
domipoppe said:
Scripters can locally modify the module (same for PW) and make such glitches easily work again.
Like call server messages clientside etc. This stuff needs to be secured and whilst releasing at as OSP it is hard to secure cause everyone can read the full code.
You see it by the NW troll bettygta etc.
You can make some glitches work by modifying client side code (generally things like wall hacks or unlimited distance name labels, which are just revealing things the client knows but hides from the player), but most should be prevented in server code: messages received by the server from a client should be mistrusted wherever possible. For example, the standard PW code for transferring money from a castle money chest has the client side check for the distance to the chest, whether there is enough money in the chest, then sends the player id, chest instance id, and money to transfer; the server side receives those values and does all the checks again and more, so the distance to the chest is checked again, the money in the chest, also whether the player is in the right faction, has keys, or the chest is broken, and then only if all the checks pass on the server is the money actually transferred and the clients updated. You might notice that the same script cf_use_castle_money_chest is run on both the clients and the server, with some code being run on both, and other bits split up: I designed various scripts this way to share code, and they should be labeled at the first line comment whether server, client, or both.

The general principle is that script checks on the client side can be useful to run preparatory code to find things - which can be computationally expensive, avoiding load on the server where possible - and (on clients not modified to remove checks) to prevent idiots spamming buttons to flood the server with impossible request messages; then every client check before requesting something is also rechecked on the server if at all possible. On the server it is not necessary to check again for every cart that is in range of the player to attach, but the nearest cart that was found on the client side and requested to attach with a network message must be within range and pass every other check in server side code before attaching. This way it doesn't generally matter if the module system is public, since players can't affect the important script checks running on the server. Another example is the PW animation scripts on the server are checked to prevent players calling them too often and being annoying, with the server enforcing a cool down period; as opposed to (if I recall correctly, at least in early days) the animation scripts for Viking Conquest multiplayer, which were not even checked whether the right agent id was sent, so people with modified clients could force other player characters to stop and perform a specific animation using malicious request messages.

My guess is that the exploit of dropping money bags with various (non official) money banking scripts is caused by not removing the money from the player on the warband server before sending the transfer request to the external web server and database: if the player tries to deposit money, the gold amount deposited should be checked and then removed immediately, probably storing the subtracted gold amount in a player slot (something like slot_player_unconfirmed_gold_amount_deposited, also a slot_player_time_gold_deposited with the current mission time), then sending the deposit message to the web server, and if the web server replies quickly enough with a successful deposit message, the player slots are cleared and the money remains unmodified, otherwise the server checks players every now and then with unconfirmed deposits a minute or so old, and refunds the undeposited money value to the player with an error message, so player gold isn't lost due to web server disconnections. Maybe if the server receives a deposit confirmation from the web server for a player without the unconfirmed slots set, the money should be subtracted from the player (in case the web server finally confirmed a deposit after the server gave up and refunded). The player should be prevented from making any further deposits or withdrawals while awating confirmation; alternatively you could only subtract the player gold after a web server transfer was confirmed, but try to make all scripts that deal with player gold fail if the player is marked as awaiting a bank transfer confirmation, so players couldn't drop money bags or buy anything until the bank transfer was confirmed to go through (or you could try overdrafts with penalty interest, jail time, or whatever other things real banks do).

Any time a check is run then a message is sent off to make a modification based on that check in another process later on, race conditions could happen; so ideally the checks should happen immediately in the M&B script before the modification is completed, which since the module system interpreter is not multi threaded - to my knowledge - should prevent that type of exploit.

Server messages or scripts run client side on a modified client should not affect the server, just desynchronize what that player sees compared with the server and everyone else: the section for handling events from the server will not run on a server, since it is inside a block starting with neg|multiplayer_is_server; so it should not affect anything if a modified client sends something like server_event_agent_equip_armor. I don't remember testing whether client scripts can run multiplayer_send_x_to_player operations to other clients, but it seems unlikely, and that would just mess up what the other player sees, not the gold, items, combat calculations, etc. stored on the server.

If you are especially paranoid, it would be possible but annoying to maintain two code bases, with one module system that includes all code and is only run on the server, and a different but compatible module system with all server scripts removed, which is built and released to clients: the released module can then only be used to connect to servers, not host another server or check the server scripts for exploits. I thought about it for PW but decided it wasn't worth the bother, since I wanted to allow anybody in the world to host a server, and to eventually release all the code publicly anyway.

I agree +1.

Well for me my scripts temporarily remove the gold of the player before the send_message_to_url and then as soon as response comes it either:
- Give the player the gold back (if the bank is full for example)
- or finally, remove it and reset the temp slot (if the deposit was valid).

That's how I do it and this is glitchable with this spam money bag as well which makes, in my opinion, no sense cause before I send a message I temp remove the gold already.

Serverside checks are really important in general. If you release your module as OSP then it is like 100-times more important. Cause people could find network messages which do not check to spam animations or equal things (as mentioned NW troll betty).

Let's just hope BL engine & scripting will be more powerful and multi-threaded to make bigger and more immersive mods than ever.
And yes, I think Warband MS is single-threaded. It does everything step-by-step.
 
domipoppe said:
Well for me my scripts temporarily remove the gold of the player before the send_message_to_url and then as soon as response comes it either:
- Give the player the gold back (if the bank is full for example)
- or finally, remove it and reset the temp slot (if the deposit was valid).

That's how I do it and this is glitchable with this spam money bag as well which makes, in my opinion, no sense cause before I send a message I temp remove the gold already.
Hmm, that is interesting: I wonder if the root cause of the "money bag spam" exploit can be fixed in your module system code, or whether it is an unfixable problem due to how certain operations happen in the engine. I remembered a change I did years ago to restrict how often players can drop money bags, seems to be one every 5 seconds, so does that code not work any more, or by "spam money bag" do you mean something other than quickly drop heaps of money bags? Is it just simply retrying the money bag drop and bank transfer as close as possible to the same time over and over, until the exploit happens? Do you know if the same method can be used with other PW props, such as to buy weapons at the same time as dropping money bags, or does it seem to be peculiar to unofficial banking scripts?
 
Vornne said:
domipoppe said:
Well for me my scripts temporarily remove the gold of the player before the send_message_to_url and then as soon as response comes it either:
- Give the player the gold back (if the bank is full for example)
- or finally, remove it and reset the temp slot (if the deposit was valid).

That's how I do it and this is glitchable with this spam money bag as well which makes, in my opinion, no sense cause before I send a message I temp remove the gold already.
Hmm, that is interesting: I wonder if the root cause of the "money bag spam" exploit can be fixed in your module system code, or whether it is an unfixable problem due to how certain operations happen in the engine. I remembered a change I did years ago to restrict how often players can drop money bags, seems to be one every 5 seconds, so does that code not work any more, or by "spam money bag" do you mean something other than quickly drop heaps of money bags? Is it just simply retrying the money bag drop and bank transfer as close as possible to the same time over and over, until the exploit happens? Do you know if the same method can be used with other PW props, such as to buy weapons at the same time as dropping money bags, or does it seem to be peculiar to unofficial banking scripts?

That's a good question. I never tried it on other props.
So maybe it's only for the unofficial banking scripts. Hard to say.
 
Vornne said:
Hmm, that is interesting: I wonder if the root cause of the "money bag spam" exploit can be fixed in your module system code, or whether it is an unfixable problem due to how certain operations happen in the engine. I remembered a change I did years ago to restrict how often players can drop money bags, seems to be one every 5 seconds, so does that code not work any more, or by "spam money bag" do you mean something other than quickly drop heaps of money bags? Is it just simply retrying the money bag drop and bank transfer as close as possible to the same time over and over, until the exploit happens? Do you know if the same method can be used with other PW props, such as to buy weapons at the same time as dropping money bags, or does it seem to be peculiar to unofficial banking scripts?

Welcome back Vornne
 
Back
Top Bottom