Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition attack #27

Open
intech opened this issue Aug 21, 2022 · 2 comments
Open

Race condition attack #27

intech opened this issue Aug 21, 2022 · 2 comments

Comments

@intech
Copy link
Member

intech commented Aug 21, 2022

There is such a problem as the race condition attack.

This problem is famous when using the ORM pattern as Active Record and Data Mapper.
We get a record for changes and change it outside the transaction, which means that we can execute precisely the same query in parallel.

database/src/methods.js

Lines 592 to 595 in 3729bf9

const entity = await this.resolveEntities(ctx, params, {
transform: false,
throwIfNotExist: true
});

result = await adapter.updateById(id, params, { raw: rawUpdate });

Consider a simple attack in transferring $5 from Alice to Bob:

Get balance Alice -> $5 -> transfer to Bob -$5 -> get balance Bob -> $0 -> received from Alice +$5

And now, if two queries were running at the same time:

Query 1: Get balance Alice -> $5 (race condition!) -> transfer to Bob -$5 -> get balance Bob -> $0 -> received from Alice +$5

Query 2: Get balance Alice -> $5 (race condition!) -> transfer to Bob -$5 -> get balance Bob -> $5 -> recieved from Alice +$5

Bob has a balance of $10, and Alice has $5.

It is elementary to check this behaviour by calling the Promise.all() method, we generate ten requests and, depending on the network, and the processing speed of the database, from 2 to 10 requests will pass simultaneously.

In SQL, the first statement must be with SELECT FOR UPDATE to block against concurrent updates, or both wrapped in a SERIALIZABLE transaction isolation level.

How can we protect ourselves now?

We must wrap the updateEntity method in a transaction with isolation level SERIALIZABLE.
Or use moleculer-channels to update items in strong FIFO in Kafka.

@icebob
Copy link
Member

icebob commented Aug 21, 2022

Do you think this issue should be solved in this module, or only in knex adapter, or outside of the lib in the application layer? How it solved in other ORMs like sequelize, Mongoose or others on other language ORMs?

@intech
Copy link
Member Author

intech commented Aug 21, 2022

TypeORM uses the lock option in the transaction
Screenshot from 2022-08-21 21-10-56

Sequelize uses the lock option in the find used version: true in models and OptimisticLockError.

Prisma description with issue and PR for query builder methods whereUnique.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants