code-423n4 / 2024-03-phala-network-findings

0 stars 0 forks source link

Deterministic Random Number Generation in CallInCommand is wrongly implemented #16

Closed c4-bot-5 closed 5 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/runtime/extension.rs#L434-L440

Vulnerability details

Proof of Concept

The implementation of the CallInCommand structure within the Pink runtime framework presents a potential security risk. The getrandom function consistently returns an empty vector, effectively generating a deterministic "random" number (zero) within a transaction context.

Take a look at https://github.com/code-423n4/2024-03-phala-network/blob/a01ffbe992560d8d0f17deadfb9b9a2bed38377e/phala-blockchain/crates/pink/runtime/src/runtime/extension.rs#L434-L440

    fn getrandom(&self, _length: u8) -> Result<Vec<u8>, Self::Error> {
        Ok(vec![])
    }

It can be seen that this function always returns an empty vector, regardless of the requested length.

Impact

This predictability can negatively impact applications that rely on getrandom for randomness within transactions:

  1. Predictable Behavior: Applications expecting a true random number might unknowingly use a constant value (zero) instead. This predictability can be exploited by attackers who can potentially predict application behavior or outputs based on this deterministic value.
  2. Weakened Cryptography: Many cryptographic algorithms and security protocols rely on strong, unpredictable randomness for key generation, encryption, and other operations. Using a constant value (zero) weakens these algorithms and makes them susceptible to attacks.

The severity of this impact depends on how applications utilize the "random" number within transactions. If critical security operations depend on this function, the consequences can be significant.

Recommended Mitigation Steps

Here are some recommendations to address this issue:

  1. Implement Secure Random Number Generation: Modify the getrandom function within CallInCommand to utilize a cryptographically secure random number generator (CSPRNG) that is suitable for the Pink runtime environment. This ensures true randomness even within transactions.

Alternatively, an attempt on this method should always error out or clearly document in the Pink runtime documentation that getrandom does not return a random number.

Assessed type

Context

c4-pre-sort commented 5 months ago

141345 marked the issue as primary issue

c4-pre-sort commented 5 months ago

141345 marked the issue as sufficient quality report

141345 commented 5 months ago

yet to implement random num generation funciton

kvinwang commented 5 months ago

It's not a deterministic RNG, as the doc says, it is only for query.

c4-sponsor commented 5 months ago

kvinwang (sponsor) disputed

c4-judge commented 5 months ago

OpenCoreCH marked the issue as unsatisfactory: Invalid