[Bug] Incorrect parsing behavior in the function input type when the identifier starts with a literal type name

[pventuzelo]

Incorrect parsing behavior in the function input type when the identifier starts with a literal type name.

Author(s): @Fuzzinglabs
Date: 06/12/2023

Executive Summary

We (@FuzzingLabs) found an incorrect parsing behavior in the function input type parsing from .aleo file.

Vulnerability Details

• Severity: Low
• Affected Component: snarkVM

Environment

• snarkVM Version: v0.16.10

The problem

This program does not compile

program mxiucxoegdphrkfqsqls.aleo;

struct i8NUUDPp9E:
    DAH1XDrG0t as u32;

function ff_zxSaOEX:
    input r0 as i8NUUDPp9E.public;

This one compile

program mxiucxoegdphrkfqsqls.aleo;

struct zi8NUUDPp9E:
    DAH1XDrG0t as u32;

function ff_zxSaOEX:
    input r0 as zi8NUUDPp9E.public;

The only change is that we changed the name of the structure from i8NUUDPp9E to zi8NUUDPp9E.
It is to be noted that according to the grammar, it is allowed to have an identifier with the name i8NUUDPp9E, it is confirmed in the code since the parsing error does not occur when parsing the structure but when parsing the input type

Steps to Reproduce

Either take the above code or this rust one

use snarkvm_console_network::Testnet3;
use snarkvm_console_program::PlaintextType;
use snarkvm_console_types_boolean::FromStr;

fn main() {
    println!("{:?}", PlaintextType::<Testnet3>::from_str("i8")); // Parse into an i8
    println!("{:?}", PlaintextType::<Testnet3>::from_str("qzd")); // Parse into an identifier
    println!("{:?}", PlaintextType::<Testnet3>::from_str("i8qzd")); // Parse into an i8, then has remaining data to parse
}

Root Cause Analysis

The root of this problem is here: https://github.com/AleoHQ/snarkVM/blob/f58905ea5b04522bf4928a331454579d569692f2/console/program/src/data_types/plaintext_type/parse.rs#L20

As said in some lines above this one, the order in which the parser is tried is important.

This creates a problem since we cannot have custom types that start with a literal type name (here i8).
This is not reflected in the grammar, nor do we know if it is possible.

Recommendations

We think that the easiest fix would be to modify the grammar to completely forbid identifiers that start with a literal type name.
This would restrict a bit more the Identifier type but avoid modifying the existing code base too much.

The problem here and why the order of the parser is important is that as of now, all the literal types are valid identifiers.

So if the parsing of an identifier before trying to parse a literal type will always result in having an identifier or an error, even when the string is a literal type

Do note that there's a check for reserved keywords, such that we cannot name a struct with just i8, maybe extend this functionality to disallow any identifier that starts with a reserved keyword