3 ms·
The core problem here is that web application developers don't understand or use database transactions. I've seen this myself at almost every web shop I've ever
by josephg 3y ago
The core problem here is that web application developers don't understand or use database transactions. I've seen this myself at almost every web shop I've ever worked at - queries / updates would be made to the database in series - "fetch this, then update that, then update this other thing". Your code works fine locally, but when multiple sessions interact with that data all at the same time, who knows what will happen.
I hadn't thought of using these race conditions to trigger security vulnerabilities. That makes all sorts of sense, and I'm going to use this work to justify my persnicketyness in future projects.
It doesn't help that popular ORMs either don't expose transactions at all or they consider transaction handling to be an advanced feature. It looks like mongodb only (finally) added transactions in mongo 4.2.
The general rule for how to avoid this problems isn't locking. Its using a database transaction per HTTP request, or "operation" - however you define that. Transactions are perfect because if any part of the request fails you can rollback the entire transaction. And if you really need to, you can emulate something similar yourself by having a "last modified" nonce on your record (a random number will do). If you ever do a read-write cycle - say from a web frontend - then the read should include reading the nonce, and the write should fail on the server if the nonce has changed since whatever value you previously read. (And you can quietly fix this by simply retrying the entire operation).
From a security point of view, this is a fantastic find. I anticipate a massive amount of code out there is at best buggy, and at worst vulnerable to these sort of attacks.
- mickeyp 3y ago> The core problem here is that web application developers don't understand or use database transactions. I've seen this myself at almost every web shop I've ever worked at - queries / updates would be made to the database in series - "fetch this, then update that, then update this other thing". Your code works fine locally, but when multiple sessions interact with that data all at the same time, who knows what will happen. I mean, you're right, and it's a bugbear of mine, too. But let's not pretend that SELECT FOR UPDATE and out-of-DB manipulation of said data fixes everything. (Nor that a direct UPDATE statement would.) Ultimately, either your updates are linearisable or they are not. Changing someone's first name with or without this methodology does not alter the fact that "last one to change wins". Which change is the right one is in this case in the eye of the beholder and the domain you work in. But, yes, updates that build on the existing values to derive new ones are common data races because people, indeed, do not understand databases, transactions or isolation levels. There are easy fixes for this type of problem. But it's never quite as simple as making everything SERIALIZABLE and FOR UPDATE and expecting that to resolve version conflicts because two different things have different expectations of what a row value should be.
- josephg 3y ago> But it's never quite as simple as making everything SERIALIZABLE and FOR UPDATE and expecting that to resolve version conflicts because two different things have different expectations of what a row value should be. Can you give some examples where making everything serializable is a poor choice? I think most of the vulnerabilities and bugs happen because people don't think about isolation / atomicity at all. And to me, SERIALIZABLE is probably the right default for 99% of regular websites. The example that comes to mind for me is realtime collaborative editing, though you can build that on top of serializable database transactions just fine. In what other situations do you need a different strategy?
- klysm 3y agoI absolutely agree that most apps should be using serializable transactions. It’s unlikely they are at a scale where it matters from a performance perspective.
- dontlaugh 3y agoSELECT FOR UPDATE is a much better default, though. Updates end up ordered by which did the update first, which tends to match intuition. Of course the callers may be surprised at being out-raced, so you still need to deal with that. Often it's enough to return the updated value.
- tzs 3y agoAre there are any relational databases, or add-ons or frontends for relational databases, that allow for transaction sharing? What I mean by "transaction sharing" is for separate connections to be simultaneously using the same transaction. Consider something whose logic is like this: begin_transaction() select_something() update_something() compute_something() update_some_more() commit_transaction() where compute_something() includes calling a service running in another process or on another server to return some data, and that service uses the same database, and we want it to see the data that update_something() updated. With transaction sharing it might instead work something like this: tid = begin_shared_transaction() select_something() update_something() compute_something(tid) update_some_more() commit_transaction() and in the service that compute_something(tid) calls it could take tid as a parameter and it would do something like open_shared_transaction(tid) ... close_transaction()
- rockostrich 3y agoI mean, it's possible to build something like that but why would you ever want to do that? It's so much simpler to just use a transaction per request and let the database's transaction logic handle which requests succeed and which ones fail. Clients should have retry logic to handle the failure scenarios. It's a lot simpler to reason about than trying to turn a distributed system into a single threaded one.
- josephg 3y agoI presume the idea is to make microservices able to work with database transactions. Microservices: Proof that not all ideas can be winners.
- rockostrich 3y agoNot really sure what the point is here. Microservices work great with database transactions.
- dontlaugh 3y ago
- johnnyworker 3y agoThanks for that! It seems trivial to enhance a DB wrapper to begin a transaction on the first query, and commit when the program finished without error. Like honestly, unless I totally missed something, why is that not built into everything it could be built into? Or put differently, is there ever a good reason to not use transactions?
- josephg 3y agoTo be clear, you want to begin a transaction at the start of each http request and submit the transaction just before sending the response. (And error or retry if the transaction fails, depending on how your database works). But generally I agree. This pattern should generally be built into everything. Whenever I’m setting up an application server I usually whip up some middleware which takes care of this for me. The code is database specific - the semantics can differ slightly depending on what database you’re using. And it only matters for requests which issue multiple commands to the database before returning. (But this happens all the time in many applications. Especially if you’re using an ORM.)
- robocat 3y agoI presume that a DB lock for the length of the request processing time would cause contention when the number of concurrent session requests increases (with user growth). The article mentions this example: Some data structures aggressively tackle concurrency issues by using locking to only allow a single worker to access them at a time. One example of this is PHP's native session handler - if you send PHP two requests in the same session at the same time, they get processed sequentially! This approach is secure against session-based race conditions but it's terrible for performance, and quite rare as a result.
- josephg 3y agoThis is a misconception about how transactions are implemented by the database. Unlike php, a lot of modern databases implement transactions without locking out other concurrent queries from being executed at the same time. In serializable (linearizable?) databases, everything happens as if the queries happened one at a time, but using magic tricks like MVCC they don’t need to lock the database to do it. It’s cool stuff! So long as your transactions are short lived it usually performs way better than you’d expect. Modern databases are incredible feats of engineering.