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

Fix dos 2723 and prevent further price related dos attacks #2724

Closed
wants to merge 15 commits into from
Closed

Fix dos 2723 and prevent further price related dos attacks #2724

wants to merge 15 commits into from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented May 7, 2022

close neo-project/neo-vm#458

fix #2723

Dynamic opcode price strategy

            var borderMax = 1024 * 1024 / (OpCodePrices[opCode] == 0 ? 1 : OpCodePrices[opCode]);
            
            // Price increase fraction
            var fraction = 3;
            // if the opcode is used for too many times
            // 100% - 110% borderMax => (1 + fraction) * price
            // 110% - 120% borderMax => (1 + 2fraction) * price
            // ...
            // 180% - 190% borderMax => (1 + 9fraction)) * price
            // >190% borderMax, exception

@Jim8y Jim8y changed the base branch from master to develop May 7, 2022 13:46
@Jim8y
Copy link
Contributor Author

Jim8y commented May 7, 2022

I did not update src/neo/SmartContract/Helper.cs

cause ordinary operations are not likely to trigger the *10 price.....to be honest, i dont think anyone other than hackers would trigger *10 price

@Jim8y Jim8y changed the title Fix dos 2723 Fix dos 2723 and prevent further dos attacks May 7, 2022
@Jim8y Jim8y changed the title Fix dos 2723 and prevent further dos attacks Fix dos 2723 and prevent further price related dos attacks May 7, 2022
@erikzhang
Copy link
Member

Why the UT fails?

@erikzhang
Copy link
Member

I don't think this solves the problem. When the count of opcodes reaches borderMax, I can send another transaction.

@roman-khimov
Copy link
Contributor

I don't think this solves the problem. When the count of opcodes reaches borderMax, I can send another transaction.

And several "problematic" opcodes can be hammered on even in a single transaction.

@erikzhang erikzhang changed the base branch from develop to master May 19, 2022 02:09
@Jim8y
Copy link
Contributor Author

Jim8y commented May 19, 2022

I don't think this solves the problem. When the count of opcodes reaches borderMax, I can send another transaction.

Reaching the BorderMax only takes very small amount of time. But yes, this can only prevent dos from a single transaction or some (a few) transactions.
If we wanna prevent DOS from block level once for all, we need to sum up the OpCodes cross transactions during preexecution.

@Jim8y
Copy link
Contributor Author

Jim8y commented May 19, 2022

And several "problematic" opcodes can be hammered on even in a single transaction.

The maxBorder is related to the price of the Opcode, if it takes longer to execute, then its maxBorder would relatively be smaller.

@erikzhang
Copy link
Member

If we wanna prevent DOS from block level once for all, we need to sum up the OpCodes cross transactions during preexecution.

But this introduces non-deterministic prices.

@Jim8y
Copy link
Contributor Author

Jim8y commented May 19, 2022

But this introduces non-deterministic prices.

We just count the number of each opcode then.

@erikzhang
Copy link
Member

I will close this pr since it doesn't solve the problem.

And neo-project/neo-vm#471 should fix #2723.

@erikzhang erikzhang closed this May 20, 2022
@Jim8y Jim8y deleted the fix-dos-2723 branch July 3, 2022 19:04
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

Successfully merging this pull request may close these issues.

GAS price for Bytes operation is too cheap: cost 37 mins with 20GAS
4 participants