pwning_dev - Error Handling in `authorize` Function could lead to dos attack

[sherlock-admin2]

pwning_dev

High

Error Handling in authorize Function could lead to dos attack

Summary

The authorize function uses impl TryInto<ProgramID<N>> andimpl TryInto<Identifier<N>> for program_id andfunction_name parameters respectively. This conversion may fail, leading to a potential panic if not properly handled. The absence of explicit error handling for these conversions can cause the entire function to panic, resulting in a crash and potential denial of service (DoS)

Vulnerability Detail

pub fn authorize<A: circuit::Aleo<Network = N>, R: Rng + CryptoRng>(
    &self,
    private_key: &PrivateKey<N>,
    program_id: impl TryInto<ProgramID<N>>,
    function_name: impl TryInto<Identifier<N>>,
    inputs: impl ExactSizeIterator<Item = impl TryInto<Value<N>>>,
    rng: &mut R,
) -> Result<Authorization<N>> {
    // Authorize the call.
    self.get_stack(program_id)?.authorize::<A, R>(private_key, function_name, inputs, rng)
}

Impact

Denial of Service (DoS): If the TryInto conversion fails, it can cause the application to panic, leading to a service interruption.

POC

use anyhow::Result;
use rand::rngs::ThreadRng;
use rand::Rng;

trait Network {}
trait Aleo: Network {}
struct PrivateKey<N: Network> { _marker: std::marker::PhantomData<N> }
impl<N: Network> PrivateKey<N> {
    fn random() -> Self {
        PrivateKey { _marker: std::marker::PhantomData }
    }
}
struct ProgramID<N: Network> { _marker: std::marker::PhantomData<N> }
impl<N: Network> TryFrom<&str> for ProgramID<N> {
    type Error = &'static str;

    fn try_from(value: &str) -> Result<Self, Self::Error> {
        if value == "valid_program_id" {
            Ok(ProgramID { _marker: std::marker::PhantomData })
        } else {
            Err("Invalid program_id")
        }
    }
}
struct Identifier<N: Network> { _marker: std::marker::PhantomData<N> }
impl<N: Network> TryFrom<&str> for Identifier<N> {
    type Error = &'static str;

    fn try_from(value: &str) -> Result<Self, Self::Error> {
        if value == "valid_function_name" {
            Ok(Identifier { _marker: std::marker::PhantomData })
        } else {
            Err("Invalid function_name")
        }
    }
}
struct Authorization<N: Network> { _marker: std::marker::PhantomData<N> }
struct Stack<N: Network> { _marker: std::marker::PhantomData<N> }
impl<N: Network> Stack<N> {
    fn authorize<A: Aleo, R: Rng>(
        &self,
        _private_key: &PrivateKey<N>,
        _function_name: Identifier<N>,
        _inputs: impl ExactSizeIterator<Item = impl TryInto<Value<N>>>,
        _rng: &mut R,
    ) -> Result<Authorization<N>> {
        Ok(Authorization { _marker: std::marker::PhantomData })
    }
}
struct Value<N: Network> { _marker: std::marker::PhantomData<N> }
struct Process<N: Network> { _marker: std::marker::PhantomData<N> }
impl<N: Network> Process<N> {
    fn new() -> Self {
        Process { _marker: std::marker::PhantomData }
    }

    fn get_stack(&self, _program_id: ProgramID<N>) -> Result<Stack<N>> {
        Ok(Stack { _marker: std::marker::PhantomData })
    }

    pub fn authorize<A: Aleo<Network = N>, R: Rng + CryptoRng>(
        &self,
        private_key: &PrivateKey<N>,
        program_id: impl TryInto<ProgramID<N>>,
        function_name: impl TryInto<Identifier<N>>,
        inputs: impl ExactSizeIterator<Item = impl TryInto<Value<N>>>,
        rng: &mut R,
    ) -> Result<Authorization<N>> {
        self.get_stack(program_id)?.authorize::<A, R>(private_key, function_name, inputs, rng)
    }
}

fn main() {
    let process = Process::new();
    let private_key = PrivateKey::<dyn Network>::random();

    // Invalid inputs for DoS attack
    let invalid_program_id = "invalid_program_id";
    let invalid_function_name = "invalid_function_name";

    let inputs = vec![];
    let mut rng = rand::thread_rng();

    // Simulate DoS attack by repeatedly calling the authorize function with invalid inputs
    for _ in 0..1000 {
        let result = process.authorize::<dyn Aleo, _>(
            &private_key,
            invalid_program_id,
            invalid_function_name,
            inputs.clone().into_iter(),
            &mut rng,
        );

        match result {
            Ok(auth) => println!("Authorization successful: {:?}", auth),
            Err(e) => eprintln!("Error: {:?}", e),
        }
    }
}

Code Snippet

https://github.com/sherlock-audit/2024-05-aleo/blob/main/snarkVM/synthesizer/process/src/authorize.rs#L20

Tool used

Manual Review

Recommendation

Add explicit error handling for TryInto conversions in the authorize function:

use anyhow::{anyhow, Result};

pub fn authorize<A: Aleo<Network = N>, R: Rng + CryptoRng>(
    &self,
    private_key: &PrivateKey<N>,
    program_id: impl TryInto<ProgramID<N>>,
    function_name: impl TryInto<Identifier<N>>,
    inputs: impl ExactSizeIterator<Item = impl TryInto<Value<N>>>,
    rng: &mut R,
) -> Result<Authorization<N>> {
    // Handle TryInto conversion errors explicitly
    let program_id = program_id.try_into().map_err(|_| anyhow!("Invalid program_id"))?;
    let function_name = function_name.try_into().map_err(|_| anyhow!("Invalid function_name"))?;

    // Authorize the call.
    self.get_stack(program_id)?.authorize::<A, R>(private_key, function_name, inputs, rng)
}

[evanmarshall]

This isn't a real issue or related to ARC-41. authorize is only called by someone initiating a transaction, not by the validators verifying the transaction. If it panics, a user will have to try to generate a transaction again. Since validators don't call this function (unless they're also making transactions as part of some script), we shouldn't have to worry about a panic here.