[PSA] Exploit for servers linked with MySQL - Developers

[QUOTE=FPtje;51608994]Demanding server owners to install and configure socket modules for something that not only can be, but already has been implemented for the things they already have installed and configured is utterly ridiculous.[/QUOTE] Which is fine but the current methods provided above leave you open to (albeit to a small) race condition in which it's still possible to get on with multiple accounts. The sockets system allows for a realtime solution which enables the servers to notify each other easily. The entire situation is stupid but I'm not the one causing it, I'm just trying to offer a better solution. Odds are the vast majority here will be running on a vps / rented dedi which gives them greater flexibility and the ability to install such things. If they're not then the fallback above is going to be their best solution.

[QUOTE=FPtje;51608994]Demanding server owners to install and configure socket modules for something that not only can be, but already has been implemented for the things they already have installed and configured is utterly ridiculous.[/QUOTE] I completely agree. You can however solve this problem using just mysql, which everyone who can encounter these problems obviously has. It's quite simple to work up a system that would prevent anyone from joining two linked servers at once. Your argument that it's a downside that players can't play on two linked servers at the same time is ridiculous btw. When would that ever be something a "normal" player would do? [QUOTE=Teddi Orange;51609005]Which is fine but the current methods provided above leave you open to (albeit to a small) race condition in which it's still possible to get on with multiple accounts. The sockets system allows for a realtime solution which enables the servers to notify each other easily. [/QUOTE] I am assuming you are talking about the fact that you could load in before the mysql query has finished? You can very easily fix that problem by kicking that player if he hasn't been "approved" in PlayerInitialSpawn. Sockets aren't a realtime solution either. [B][I]Especially [/I][/B] if those two servers are not hosted on the same machine.

[URL]https://github.com/FPtje/DarkRP/blob/353dc3286092cdb2efade2cc0f2145db5a6c2e69/gamemode/modules/money/sv_money.lua#L7[/URL] Wouldn't a select (and doing the necessary transaction + locking + update) instead of retrieving the current amount using getDarkRPVar prevent people from duplicating money, albeit still displaying the wrong amount on the client? Would my servers be save using this method?

[QUOTE=zoox;51609076][URL]https://github.com/FPtje/DarkRP/blob/353dc3286092cdb2efade2cc0f2145db5a6c2e69/gamemode/modules/money/sv_money.lua#L7[/URL] Wouldn't a select (and doing the necessary transaction + locking + update) instead of retrieving the current amount using getDarkRPVar prevent people from duplicating money, albeit still displaying the wrong amount on one of the clients? Would my servers be save using this method?[/QUOTE] Yes there are transactional solutions to the problem. But you have to think further than that. It's not just the darkrp money system that has these problems. As the OP stated already Pointshop has a similar vulnerability. Pretty much any darkrp addon that uses mysql will have some of these issues and you can't expect everyone to implement transactional solutions (they are not trivial). So keeping that in mind I think the only good solution is preventing this scenario from happening at all.

[QUOTE=syl0r;51609090]Yes there are transactional solutions to the problem. But you have to think further than that. It's not just the darkrp money system that has these problems. As the OP stated already Pointshop has a similar vulnerability. Pretty much any darkrp addon that uses mysql will have some of these issues and you can't expect everyone to implement transactional solutions (they are not trivial). So keeping that in mind I think the only good solution is preventing this scenario from happening at all.[/QUOTE] It's not about transactions if I understood correctly. Basicly player joins Server A and B, while having his Money set to say 1 000 000 in the database. Both servers download this value when the player joins and store it [B]locally[/B] inside their memory. Then the player drops 1 000 000 money on the ground, making server A update the database and it's internal memory with the money value of 0, while server B still stores a value of 1 000 000, and so when the player drops 1$ on server B it updates the database with a new value of 999 999. Transactional solutions were made for another purpose, a very specific one, to ensure that [B]many operations[/B] would go through in one 'run', without anything else touching the values it uses while the transaction is being made, and to be able to revert it easily. In my [sp]not so educated[/sp] opinion the proper way of dealing with this would be to have the database call back to all the servers that have the player active, and updating values on servers accordingly, and since i do not know any way MySQL could handle that a 'proxy' server that connects to all Gmod servers and passess commands through to the database with knowledge of all players connected would have been created, allowing for one player to be playing at many servers without breaking shit. And please, do correct me if i'm terribly wrong. But then, what sane human would play on two servers at once?

What's wrong with doing this? 1.) Get current value from the database 2.) Use that value to do your operation, or to deny it if not possible 3.) Update the database with the new value, when changed 4.) Update shown value with the new value It's effectively preventing the exploit and possible other bugs. I think it's the only way to solve it, without dropping the multirun support. However there is still a small race condition if you are unable to lock the value while select-updating. But it would be still smaller than the one caused by using the "one select on spawn"-method. The player would have to cause two update operation at the same time, which be pretty hard if not impossible. If you worry about performance, I don't think that it is a big problem, as mysql operations doesn't happen at every think or something.

[QUOTE=Amic;51609130]It's not about transactions if I understood correctly.[/QUOTE] Obviously the problem isn't caused by transactions. One solution to the problem is using transactions (a single query also forms also a transaction btw.). Whenever the player tries to buy something it could be done like this (pseudocode) [CODE] transactionStart local money = getMoneyFromDB() if (money > x) then -- do whatever end transactionEnd [/CODE] Now unfortunately exactly that solution isn't possible in lua since tmysql and mysqloo are both async and you should never wait for the results of a query in the main server thread. You could still implement a similar solution that'd effectively do the same in the callbacks but it'd be extremely complicated to do. [QUOTE=Amic;51609130] In my [sp]not so educated[/sp] opinion the proper way of dealing with this would be to have the database call back to all the servers that have the player active, and updating values on servers accordingly[/QUOTE] There is no such thing. The solution that databases offer for these problems are [B]transactions[/B]. [QUOTE=Grocel;51609234]What's wrong with doing this? 1.) Get current value from the database 2.) Use that value to do your operation, or to deny it if not possible 3.) Update the database with the new value, when changed 4.) Update shown value with the new value[/QUOTE] As I already said multiple times that is one way to solve it but it's not universal. You'd need a seperate solution for each and every darkrp addon that makes use of mysql and let's face it, that's not gonna happen. So the only acceptable way is to prevent this from happening.

[QUOTE=Grocel;51609234]What's wrong with doing this? 1.) Get current value from the database 2.) Use that value to do your operation, or to deny it if not possible 3.) Update the database with the new value, when changed 4.) Update shown value with the new value It's effectively preventing the exploit and possible other bugs. I think it's the only way to solve it, without dropping the multirun support. However there is still a small race condition if you are unable to lock the value while select-updating. But it would be still smaller than the one caused by using the "one select on spawn"-method. The player would have to cause two update operation at the same time, which be pretty hard if not impossible. If you worry about performance, I don't think that it is a big problem, as mysql operations doesn't happen at every think or something.[/QUOTE] seem like [URL="http://stackoverflow.com/a/29849153"]SELECT ... FOR UPDATE[/URL] would prevent a race condition(?)

[QUOTE=zoox;51609278]seem like [URL="http://stackoverflow.com/a/29849153"]SELECT ... FOR UPDATE[/URL] would prevent a race condition(?)[/QUOTE] No, as the documentation states the FOR UPDATE clause requires the statement to be inside a transaction (autocommit off).

[QUOTE=syl0r;51609301]No, as the documentation states the FOR UPDATE clause requires the statement to be inside a transaction (autocommit off).[/QUOTE] I'm not at my computer right now so I can't test it - But do you think putting it in a stored procedure might work? (Provided that tmysql supports calling stored procedures in the first place)

[QUOTE=syl0r;51609022]I completely agree. You can however solve this problem using just mysql, which everyone who can encounter these problems obviously has. It's quite simple to work up a system that would prevent anyone from joining two linked servers at once. Your argument that it's a downside that players can't play on two linked servers at the same time is ridiculous btw. When would that ever be something a "normal" player would do?[/QUOTE] I was talking about MySQL when referring to the thing that had already been implemented. The argument that players can't play on two servers at once isn't an argument against this way of solving it. It's an argument against implementing it as a form of defensive programming. This shit only became a problem when multirun was allowed on sv_lan 0 servers. The argument is purely to counter Teddi's (edit: correction, it was [url=https://facepunch.com/showthread.php?t=1547100&p=51604790&viewfull=1#post51604790]slayer's[/url]) statement that developers who hadn't thought of this in the first place [u]messed up[/u]. You shouldn't implement countermeasures with such downsides to protect against things you can reasonably assume not to be possible. Transactions could be a potential solution for e.g. DarkRP money. - Rewrite canAfford to use the DB - Rewrite addMoney to update the money in a transaction. Combine with canAfford check in the transaction to prevent negative balance. Update the player's money with the latest version from the database. - Setting the money just directly sets the money in the database. This shouldn't cause conflicts with addMoney or other setMoneys. It's essentially just a problem of concurrent updates to the same record. This problem is generic, and transactions are a real solution for it. The problem is that this would require API changes (e.g. callbacks in canAfford) in DarkRP. It would either break tons of addons or leave them vulnerable. Fuck that shit. I'd rather reimplement the features provided by the addon posted in OP. Probably with a config option so you can turn it off for development purposes. Multirun is for developing, I don't want DarkRP to disable that on MySQL servers with a common database.

I wasn't suggesting anyone who hadn't thought of this had messed up and I'm not sure where you're getting that from. All I've been suggesting is what I believed to be a better solution. Hell I even suggest [url=https://facepunch.com/showthread.php?t=1547100&p=51609005&viewfull=1#post51609005]here[/url] that if they're not able to use sockets then they use the provided addon.

[QUOTE=Teddi Orange;51609702]I wasn't suggesting anyone who hadn't thought of this had messed up and I'm not sure where you're getting that from. All I've been suggesting is what I believed to be a better solution. Hell I even suggest [url=https://facepunch.com/showthread.php?t=1547100&p=51609005&viewfull=1#post51609005]here[/url] that if they're not able to use sockets then they use the provided addon.[/QUOTE] Sorry, it was [url=https://facepunch.com/showthread.php?t=1547100&p=51604790&viewfull=1#post51604790]slayer[/url]. My mistake.

Made an update to the CRCing done as [URL="https://github.com/Facepunch/garrysmod-issues/issues/3001"]game.GetIPAddress() is dumb[/URL] and likely would've only worked where ports were different and not IPs, so update that. Also made a branch for anyone still using tmysql3. It just relies on your Lua state already having a connection established with tmysql3 and piggybacks off of it.

[QUOTE=KingofBeast;51612003]Made an update to the CRCing done as [URL="https://github.com/Facepunch/garrysmod-issues/issues/3001"]game.GetIPAddress() is dumb[/URL] and likely would've only worked where ports were different and not IPs, so update that. Also made a branch for anyone still using tmysql3. It just relies on your Lua state already having a connection established with tmysql3 and piggybacks off of it.[/QUOTE] The IP wouldn't work that well if you use two diffirent proxies for the 2 clients running. *Edit, my mistake. I misunderstood what game.GetIPAddress() does.

[QUOTE=KingofBeast;51612003]Made an update to the CRCing done as [URL="https://github.com/Facepunch/garrysmod-issues/issues/3001"]game.GetIPAddress() is dumb[/URL] and likely would've only worked where ports were different and not IPs, so update that. Also made a branch for anyone still using tmysql3. It just relies on your Lua state already having a connection established with tmysql3 and piggybacks off of it.[/QUOTE] Why are you CRCing the IP? Also, the ip convar can be changed through command line arguments. I'd rather not trust that.

[QUOTE=FPtje;51613972]Why are you CRCing the IP? Also, the ip convar can be changed through command line arguments. I'd rather not trust that.[/QUOTE] Why would a server owner change the IP convar except to bind to a different IP? It CRCs IP:port to create an integer server ID for the database to use, since you won't have another server on the same IP AND port

[QUOTE=KingofBeast;51614132]It CRCs IP:port to create an integer server ID for the database to use, since you won't have another server on the same IP AND port[/QUOTE] This is UniqueID all over again. DON'T CRC IT. Why not use the actual IP and port? 48 bits, easily fits in an int64 and even doubles can hold it in lua.

[QUOTE=NeatNit;51614181]This is UniqueID all over again. DON'T CRC IT. Why not use the actual IP and port? 48 bits, easily fits in an int64 and even doubles can hold it in lua.[/QUOTE] Consider that we're comparing community servers to players. You won't have tens/hundreds of thousands of servers. The likelihood of colliding the CRCs for a handful of servers is slim.

[QUOTE=KingofBeast;51614132][B]Why would a server owner change the IP convar except to bind to a different IP?[/B] It CRCs IP:port to create an integer server ID for the database to use, since you won't have another server on the same IP AND port[/QUOTE] In scenarios such as mine where your dedicated server has several ip addresses, and each virtual server on that dedi has two (or more) network adapters, one for an internal network and one (or more) for an external network, so as to prevent SRCDS from guessing wrong and trying to establish a server on the internal network.

[QUOTE=KingofBeast;51614222]Consider that we're comparing community servers to players. You won't have tens/hundreds of thousands of servers. The likelihood of colliding the CRCs for a handful of servers is slim.[/QUOTE] I know that, but for the sake of good practice, might as well do it right. Edit: if I did [url=http://www.wolframalpha.com/input/?i=1+-+(product+of+(2%5E32%2B1-n)%2F2%5E32+for+n+%3D+1+to+10)]my math[/url] right, if you have 10 servers, the chance of a collision is 1 in 100 million. If you have 1000 servers, it's only 1 in 10,000. Birthday problem.

[QUOTE=KingofBeast;51614222]Consider that we're comparing community servers to players. You won't have tens/hundreds of thousands of servers. The likelihood of colliding the CRCs for a handful of servers is slim.[/QUOTE] Imagine having two servers that hit the one in a million chance. Fuck storage space, do it right. Also, this shit is now in the latest DarkRP.

[QUOTE=Redfiend;51614348]In scenarios such as mine where your dedicated server has several ip addresses, and each virtual server on that dedi has two (or more) network adapters, one for an internal network and one (or more) for an external network, so as to prevent SRCDS from guessing wrong and trying to establish a server on the internal network.[/QUOTE] Okay, where would that actually make this behave abnormally though? You use the ip switch to designate an IP, it's still going to be a unique IP+port combination.

[QUOTE=FPtje;51614536]Also, this shit is now in the latest DarkRP.[/QUOTE] You should be selecting where serverid != currentserverid, and once you make that change you'll have to account for what happens when someones server crashes and doesn't come back online otherwise players won't be able to join their other servers.

[QUOTE=Teddi Orange;51608985]Pass a (secret) string around that your servers use to authenticate alongside the SteamID.[/QUOTE] And how do you propose I keep this string secret? (I seriously hope you don't say "just use RSA lol, it's trivial") It's not even an unreasonable attack, if I know a some community has their servers with NFO, I could buy some hosting with them, set up my favorite packet analyzer, and if I don't get lucky, cancel and and buy another subscription until I do. [QUOTE=Grocel;51609234] If you worry about performance, I don't think that it is a big problem, as mysql operations doesn't happen at every think or something.[/QUOTE] This could be quite a serious problem, especially if your SQL server is not on the same machine as your Garry's Mod server. For example, if you have 1 US, 1 Europe, 1 East Asia server, all going to one database. Reasonable if you're just loading player data when they join, and writing it back when they leave, seriously unreasonable if you need to keep track of a player's inventory, and you expect players to move items around their inventory a lot. It seems to me these various solutions are interesting academic exercises at best. Isn't the obvious solution to put this in as a convar in the dedicated server?

[QUOTE=NeatNit;51614513]I know that, but for the sake of good practice, might as well do it right. Edit: if I did [url=http://www.wolframalpha.com/input/?i=1+-+(product+of+(2%5E32%2B1-n)%2F2%5E32+for+n+%3D+1+to+10)]my math[/url] right, if you have 10 servers, the chance of a collision is 1 in 100 million. If you have 1000 servers, it's only 1 in 10,000. Birthday problem.[/QUOTE] Who has 1000 servers? Or even 100 servers for that matter? [editline]3rd January 2017[/editline] I just hope FP Devs won't disable -multirun, used it for a long time and don't want to see it gone.

Ideally SrcDS needs to be able to see if the user already has a Steam validation created elsewhere and reject them based on a convar from there.

[QUOTE=Apickx;51616508]And how do you propose I keep this string secret? (I seriously hope you don't say "just use RSA lol, it's trivial") It's not even an unreasonable attack, if I know a some community has their servers with NFO, I could buy some hosting with them, set up my favorite packet analyzer, and if I don't get lucky, cancel and and buy another subscription until I do. This could be quite a serious problem, especially if your SQL server is not on the same machine as your Garry's Mod server. For example, if you have 1 US, 1 Europe, 1 East Asia server, all going to one database. Reasonable if you're just loading player data when they join, and writing it back when they leave, seriously unreasonable if you need to keep track of a player's inventory, and you expect players to move items around their inventory a lot. It seems to me these various solutions are interesting academic exercises at best. Isn't the obvious solution to put this in as a convar in the dedicated server?[/QUOTE] Just Imple- oh wait, you beat me to it. /s That's for you to figure out, I don't know your setup or what you have available. There are various ways you can secure against it, but it's dependant on per-environment variables. Edit: If you want something that'll handle encryption for you - [url]https://github.com/danielga/gm_crypt[/url]

[QUOTE=Teddi Orange;51607081]Again, with sockets it's easy code That should be enough to get you started. I'd recommend passing a token around as well as UDP isn't secure enough on its own.[/QUOTE] Your socket example uses UDP, which is far as as I know unreliable. - Do you send the packets containing the steamid periodically, so it doesn't really matter when a packet gets dropped? Wouldn't TCP in that case be a better choice though? + if you are using a linux server you can secure the connection with ssh tunnels.

I really don't see why you would implement this with sockets when (almost) all servers that are affected by this are servers that use a database to sync data. So all of them would have a mysql module. Additionally your socket implementation isn't even safe. You are using UDP when this is clearly a terrible choice in this case. There is literally no guarantee that the UDP packet will ever be received by your server so there is still a way that players can join. (zoox beat me to it...) If you scale this up to more than two servers being linked you'll end up having to ask every other server if the player can join while it is always one transaction in the mysql version. The likelihood of even one of those requests failing is then a problem.