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

Miners can re-roll the Chainlink VRF output #273

Open
code423n4 opened this issue Sep 27, 2022 · 8 comments
Open

Miners can re-roll the Chainlink VRF output #273

code423n4 opened this issue Sep 27, 2022 · 8 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/d2087c5a8a6a4f1b9784520e7fe75afa3a9cbdbe/src/utils/rand/ChainlinkV1RandProvider.sol#L62-L70

Vulnerability details

Miners can rewrite a chain's history if they dislike the VRF output used by the protocol. Consider the following example:

  • A miner or well-funded user mints Gobblers and intends to reveal the Gobblers to have Gobblers with a high emission multiplier
  • A Chainlink VRF request is made and fulfilled in the same block.
  • The protocol participant does not benefit from the VRF output and therefore wants to increase their chances of a high-yield goo Gobbler by including the VRF output in another block, producing an entirely new VRF output. This is done by re-orging the chain, i.e. following a new canonical chain where the VRF output has not been included in a block.
    This attack can be continued as long as the attacker controls 51% of the network. The miner itself could control a much smaller proportion of the network and still be able to mine a few blocks in succession, although this is of low probability but entirely possible.
  • A well-funded user could also pay miners to re-org the chain on their behalf in the form of MEV to achieve the same benefit.

For more details, see https://docs.chain.link/docs/vrf/v1/security/.

Impact

Miners can rewrite a chain's history if they dislike the VRF output used by the protocol.

Proof of Concept

utils/rand/ChainlinkV1RandProvider.sol

Tools Used

Manual review

Recommended mitigation steps

Consider adding a confirmation time between when the actual VRF request was made and when it was later fulfilled on-chain. This could be as few as 5 blocks, reducing the probability of an effective chain reorganization to extremely close to 0.


Disclosure: This finding has been reported in a previous Code4rena contest. For details, see code-423n4/2021-10-pooltogether-findings#56

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Sep 27, 2022
code423n4 added a commit that referenced this issue Sep 27, 2022
@Shungy
Copy link
Member

Shungy commented Sep 27, 2022

Not a thing anymore I guess? https://www.paradigm.xyz/2021/07/ethereum-reorgs-after-the-merge

@GalloDaSballo
Copy link
Collaborator

I'd like to go on the record and say that I would re-rate the Pool Together finding as Med today, as it's conditional on the VRF operator being malicious / gaining an advantage from it

@GalloDaSballo
Copy link
Collaborator

Screenshot 2022-10-10 at 00 54 23

I'll go check the source code for VRF because if the only thing Miners can do is delay the VRF fulfilment then this should not be considered logically equivalent to re-rolling until the right VRF is generated.

I also will keep in mind that there's only 4 values that can be gained from this as the emission multiple is non random and instead falls into a discrete distribution

@GalloDaSballo
Copy link
Collaborator

I've asked CL directly for confirmation

@GalloDaSballo
Copy link
Collaborator

While I'm going to continue reading the CL codebase, I believe that there's an aspect related to the predictability of the VRF output

And then the other variable has to do with the impact.

Specifically, per the discussion we've had on #125, we know that the emission multiples for Art Gobblers are non-random, and the amount is purposefully deterministic (the distribution is deterministic, the outcome of each purchase is random but contained in the distribution).

For those reasons I believe the finding will be further downgraded in severity, as it is possible (due to the delay, which influenced the blockhash, and because the blockhash can be changed via spam tx), to re-roll the dice to their favour., however impact is reduced by the deterministic distribution, and the minute advantage of the multiple which are fairly close to each other.

The specific thing I've yet to verify is if a operator or miner can pre-determine the VRF output given a specific blockhash, meaning that a Miner (and to some extend anyone) could influence the output of the VRF as it's deterministically derived from the blockhash at block X (delay which may or may not be known).

I don't think this aspect will have much impact in judging the severity of this finding, however this would help in judging a similar finding in the future

@GalloDaSballo GalloDaSballo added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 11, 2022
@GalloDaSballo
Copy link
Collaborator

I've verified within the CL source code the following facts:

A CL node will have it's own PK to sign with:
https://github.com/smartcontractkit/chainlink/blob/7d53c4609fb74aea4fe4d3e471e814c3cf1d86dd/core/services/keystore/master.go#L238-L239

type keyManager struct {

Because of that, the implementation of VRF will use the Blockhash + the unknown PK from the CL node, to generate a random value and broadcast additonal data to verify that it wasn't tampered:
https://github.com/smartcontractkit/chainlink/blob/7d53c4609fb74aea4fe4d3e471e814c3cf1d86dd/core/services/pipeline/task.vrf.go#L87-L88

	p, err := t.keyStore.GenerateProof(pk.String(), finalSeed)

Because the PK of the node cannot be known upfront (and arguably 1/X nodes could be chosen for the task), the only thing that could be tampered with is the Blockhash of the specific node.

However, because the PK is unknown, the only rational attack that can be implemented is a re-org attack, because the only time in which the VRF outcome is know is when it's broadcasted, then reorg becomes the only possible attack.

I do believe that because of the above, a theoretical attack which requires having knowledge of the CL node PK + the target block for randomness, would allow to edit the blockhash of the target block to achieve a specific target randomness value.

However, in the in-scope code, the worst case outcome for the above is the ability to achieve a higher emission multiple, which as we've argued on reports such as #254 is a Low Severity finding.

While I don't believe I've ruled out any risk in using VRF (arguably have shown the exact way to exploit it), I believe the impact in this case is at worst a Low Severity finding, and will downgrade to such

@GalloDaSballo
Copy link
Collaborator

L

@GalloDaSballo
Copy link
Collaborator

After further discussion with C4 Wardens we've verified that the outcome of the roll is deterministic, in that the combination of pk and blockhash enables to verify the roll, meaning that the source of randomness is the blockhash.

This may be useful for judging future findings.

The last piece of consideration is the fact that the delay is not verifiable with the in-scope code, so it may be best to keep these types of findings in QA Reports, as we have no way of falsifying nor verifying the settings before the contracts are deployed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

3 participants