The Storage.get method does not handle Err correctly, which may cause the node to crash.

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/main/phala-blockchain/crates/pink/runtime/src/storage/mod.rs#L115-L119

Vulnerability details

Impact

This bug may crash the node. (Denial of service)

Proof of Concept

The vulnerability in the provided code is located in the get(plala-blockchain/crates/pink/runtime/src/storage/mod.rs) function, specifically in the section that handles the get logic. Here, if the incoming key does not exist in the storage, Err will be returned, but the code does not handle Err correctly but uses except, which may cause the node to panic.

/// Gets the storage value of a specified key.
    pub fn get(&self, key: &[u8]) -> Option<Vec<u8>> {
        self.backend
            .as_trie_backend()
            .storage(key) // <- here
            .expect("Failed to get storage key") 
    }

The panic caused by .expect() is not caught, leading to a potential denial of service (DoS) as it could halt the processing of transactions or even crash the node depending on how the runtime handles panics.

Tools Used

Manual review

Recommended Mitigation Steps

To fix this vulnerability, we should handle the possibility that key can't be found in storage:

/// Gets the storage value of a specified key.
    pub fn get(&self, key: &[u8]) -> Option<Vec<u8>> {
        if let Some(value) = self.backend.as_trie_backend().storage(key) {
            // do return
        } else {
            // Handle the case where key not in storage
            // For example, log an error or take other appropriate actions
        }
    }

Assessed type

DoS

[c4-pre-sort]

141345 marked the issue as duplicate of #55

[c4-judge]

OpenCoreCH marked the issue as nullified