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

Add security page for developing applications on Sapphire/OPL #509

Open
aefhm opened this issue Aug 25, 2023 · 2 comments
Open

Add security page for developing applications on Sapphire/OPL #509

aefhm opened this issue Aug 25, 2023 · 2 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request examples Fixes in the examples code help wanted Extra attention is needed p:1 Medium priority

Comments

@aefhm
Copy link
Contributor

aefhm commented Aug 25, 2023

Why

We would like to increase advisory support for dApp developers on Sapphire/OPL and improve access to confidentiality based on conversations with @CedarMist and @oasisprotocol/documentation

What

We would like to see recommendations in the following areas.

Gas Usage

In which situations would gas usage information reveal sensitive information?
Are user accounts private?
Are user balances private?
What is private/sensitive information?
Is the private/sensitive information conditionally so?
What options do developers or users have surrounding this information?

Storage I/O Patterns

Could we use hardhat-tracer or some tool to examine calls?
Do we need padding around size of storage keys/values?
Do we need ORAM?

Parameter Size

In general, privacy sensitive functions should have the same call data size.

RNG

How are precompiles different from VRFs? Maybe we have to prevent reversion by smart contract and implement a singular block delay. Is there modulo bias. Should dApp developers introduce additional entropy through block hash, VRF from other chains, or arbitrary user randomness? Is there code path reuse of the same random number for different purposes?

Transparent EVM vs Confidential EVM

Events are public. Private data syncing may need encrypted events. Potentially use view calls to query state. Private data must be queried using 'view' calls to the smart contract.

Normal observers can see both which account performed a transaction and which contract they interacted with as well as the size of the call data.

Developers may want to make private which function is called by ensuring that all privacy sensitive functions have the same length call data with 'empty' parameters padded.

Error Order

If one error occurs before another, gas usage patterns can reveal which code path was taken.
If the check/condition relies on sensitive data (specific to the contracts own privacy requirements) there are a number of ways to brute force the value.

Example 1 (realistic):
Balance check happens before checking if user is allowed to perform transfer
Gas usage for the 2 different code paths is different
Attacker checks e.g. $100 = code path 2 (user has sufficient balance but user not authorized)
Attacker checks $1000 = code path 1 (user doesn't have sufficient balance)
Attacker knows users balance is between 100 and 1000, with more tests can narrow it down to near-exact value in small number of steps.

Solution:
Move 'isApproved' check above 'hasSufficientBalance' because somebody who is approved to spend needs to know the balance.

@aefhm aefhm added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed p:1 Medium priority examples Fixes in the examples code labels Aug 25, 2023
@aefhm aefhm self-assigned this Aug 25, 2023
@CedarMist
Copy link
Member

CedarMist commented Aug 29, 2023

Error Order:

Reveals balance of who to anybody, via binary search of amount using gas usage:

function transferFrom(address who, address to, uint amount)
  external
{
  require( balances[who] >= amount );
  require( allowances[who][msg.sender] >= amount );
}

Doesn't reveal the information unless the account is authorized to spend, will not reveal the full balance - only what is authorized.

function transferFrom(address who, address to, uint amount)
  external
{
  require( allowances[who][msg.sender] >= amount );
  require( balances[who] >= amount );
}

@aefhm
Copy link
Contributor Author

aefhm commented Nov 7, 2023

Let's also add section to consider using block heights to ensure finality. https://discord.com/channels/748635004384313474/960599114993762304/1167175003083657287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request examples Fixes in the examples code help wanted Extra attention is needed p:1 Medium priority
Projects
None yet
Development

No branches or pull requests

2 participants