Skip to content

Commit

Permalink
Verify account space specified in annotation (#1531)
Browse files Browse the repository at this point in the history
On Solana, we can specify the space necessary for a contract's data
account in the constructor with an annotation. We do not verify,
however, if the declared space is enough to hold all the contract
variables. As a consequence, we could call the constructor without any
issue using a very small space value and hit other runtime errors later.

Signed-off-by: Lucas Steuernagel <[email protected]>
  • Loading branch information
LucasSte authored Sep 26, 2023
1 parent d91fe95 commit 614d413
Show file tree
Hide file tree
Showing 13 changed files with 366 additions and 209 deletions.
38 changes: 20 additions & 18 deletions src/bin/languageserver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1409,25 +1409,27 @@ impl<'a> Builder<'a> {
continue;
}

for note in &func.annotations {
match note {
ast::ConstructorAnnotation::Bump(expr)
| ast::ConstructorAnnotation::Seed(expr)
| ast::ConstructorAnnotation::Space(expr) => {
builder.expression(expr, &func.symtable)
}
if let Some(bump) = &func.annotations.bump {
builder.expression(&bump.1, &func.symtable);
}

ast::ConstructorAnnotation::Payer(loc, name) => {
builder.hovers.push((
loc.file_no(),
HoverEntry {
start: loc.start(),
stop: loc.exclusive_end(),
val: format!("payer account: {name}"),
},
));
}
}
for seed in &func.annotations.seeds {
builder.expression(&seed.1, &func.symtable);
}

if let Some(space) = &func.annotations.space {
builder.expression(&space.1, &func.symtable);
}

if let Some((loc, name)) = &func.annotations.payer {
builder.hovers.push((
loc.file_no(),
HoverEntry {
start: loc.start(),
stop: loc.exclusive_end(),
val: format!("payer account: {name}"),
},
));
}

for (i, param) in func.params.iter().enumerate() {
Expand Down
28 changes: 28 additions & 0 deletions src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ use std::cmp::Ordering;
use crate::codegen::cfg::ASTFunction;
use crate::codegen::solana_accounts::account_management::manage_contract_accounts;
use crate::codegen::yul::generate_yul_function_cfg;
use crate::sema::diagnostics::Diagnostics;
use crate::sema::eval::eval_const_number;
use crate::sema::Recurse;
#[cfg(feature = "wasm_opt")]
use contract_build::OptimizationPasses;
use num_bigint::{BigInt, Sign};
use num_rational::BigRational;
use num_traits::{FromPrimitive, Zero};
use solang_parser::diagnostics::Diagnostic;
use solang_parser::{pt, pt::CodeLocation};

// The sizeof(struct account_data_header)
Expand All @@ -56,6 +59,10 @@ pub const SOLANA_FIRST_OFFSET: u64 = 16;
/// Name of the storage initializer function
pub const STORAGE_INITIALIZER: &str = "storage_initializer";

/// Maximum permitted size of account data (10 MiB).
/// https://github.com/solana-labs/solana/blob/08aba38d3507c8cb66f85074d8f1249d43e64a75/sdk/program/src/system_instruction.rs#L85
pub const MAXIMUM_ACCOUNT_SIZE: u64 = 10 * 1024 * 1024;

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum OptimizationLevel {
None = 0,
Expand Down Expand Up @@ -330,6 +337,27 @@ fn layout(contract_no: usize, ns: &mut Namespace) {
}
}

let constructors = ns.contracts[contract_no].constructors(ns);
if !constructors.is_empty() {
if let Some((_, exp)) = &ns.functions[constructors[0]].annotations.space {
// This code path is only reachable on Solana
assert_eq!(ns.target, Target::Solana);
if let Ok((_, value)) = eval_const_number(exp, ns, &mut Diagnostics::default()) {
if slot > value {
ns.diagnostics.push(Diagnostic::error(
exp.loc(),
format!("contract requires at least {} bytes of space", slot),
));
} else if value > BigInt::from(MAXIMUM_ACCOUNT_SIZE) {
ns.diagnostics.push(Diagnostic::error(
exp.loc(),
"Solana's runtime does not permit accounts larger than 10 MB".to_string(),
));
}
}
}
}

ns.contracts[contract_no].fixed_layout_size = slot;
}

Expand Down
2 changes: 1 addition & 1 deletion src/codegen/revert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ pub(crate) fn error_msg_with_loc(ns: &Namespace, error: String, loc: Option<Loc>
}
}

fn string_to_expr(string: String) -> Expression {
pub(super) fn string_to_expr(string: String) -> Expression {
Expression::FormatString {
loc: Loc::Codegen,
args: vec![(
Expand Down
132 changes: 79 additions & 53 deletions src/codegen/solana_deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ use super::{
cfg::ReturnCode, expression, Builtin, ControlFlowGraph, Expression, Instr, Options, Type,
Vartable,
};
use crate::codegen::revert::string_to_expr;
use crate::codegen::solana_accounts::account_management::{
account_meta_literal, retrieve_key_from_account_info,
};
use crate::sema::ast::{
self, ArrayLength, CallTy, ConstructorAnnotation, Function, FunctionAttributes, Namespace,
StructType,
self, ArrayLength, CallTy, Function, FunctionAttributes, Namespace, StructType,
};
use crate::sema::diagnostics::Diagnostics;
use crate::sema::eval::eval_const_number;
use crate::sema::solana_accounts::BuiltinAccounts;
use base58::ToBase58;
use num_bigint::{BigInt, Sign};
Expand Down Expand Up @@ -78,20 +80,12 @@ pub(super) fn solana_deploy(

cfg.set_basic_block(id_fail);

let message = format!("program_id should be {}", program_id.to_base58()).into_bytes();

let expr = Expression::AllocDynamicBytes {
loc: Loc::Codegen,
ty: Type::String,
size: Box::new(Expression::NumberLiteral {
loc: Loc::Codegen,
ty: Type::Uint(32),
value: BigInt::from(message.len()),
}),
initializer: Some(message),
};

cfg.add(vartab, Instr::Print { expr });
cfg.add(
vartab,
Instr::Print {
expr: string_to_expr(format!("program_id should be {}", program_id.to_base58())),
},
);

cfg.add(
vartab,
Expand Down Expand Up @@ -249,11 +243,7 @@ pub(super) fn solana_deploy(
}
}

if let Some(ConstructorAnnotation::Payer(_, name)) = func
.annotations
.iter()
.find(|tag| matches!(tag, ConstructorAnnotation::Payer(..)))
{
if let Some((_, name)) = &func.annotations.payer {
let metas_ty = Type::Array(
Box::new(Type::Struct(StructType::AccountMeta)),
vec![ArrayLength::Fixed(BigInt::from(2))],
Expand Down Expand Up @@ -314,13 +304,57 @@ pub(super) fn solana_deploy(
);

// Calculate minimum balance for rent-exempt
let (space, lamports) = if let Some(ConstructorAnnotation::Space(space_expr)) = func
.annotations
.iter()
.find(|tag| matches!(tag, ConstructorAnnotation::Space(..)))
{
let space_var = vartab.temp_name("space", &Type::Uint(64));
let (space, lamports) = if let Some((_, space_expr)) = &func.annotations.space {
let expr = expression(space_expr, cfg, contract_no, None, ns, vartab, opt);
// If the space is not a literal or a constant expression,
// we must verify if we are allocating enough space during runtime.
if eval_const_number(space_expr, ns, &mut Diagnostics::default()).is_err() {
let cond = Expression::MoreEqual {
loc: Loc::Codegen,
signed: false,
left: Box::new(expr.clone()),
right: Box::new(Expression::NumberLiteral {
loc: Loc::Codegen,
ty: Type::Uint(64),
value: contract.fixed_layout_size.clone(),
}),
};

let enough = cfg.new_basic_block("enough_space".to_string());
let not_enough = cfg.new_basic_block("not_enough_space".to_string());

cfg.add(
vartab,
Instr::BranchCond {
cond,
true_block: enough,
false_block: not_enough,
},
);

cfg.set_basic_block(not_enough);
cfg.add(
vartab,
Instr::Print {
expr: string_to_expr(format!(
"value passed for space is \
insufficient. Contract requires at least {} bytes",
contract.fixed_layout_size
)),
},
);

cfg.add(
vartab,
Instr::ReturnCode {
code: ReturnCode::AccountDataTooSmall,
},
);

cfg.set_basic_block(enough);
}

let space_var = vartab.temp_name("space", &Type::Uint(64));

cfg.add(
vartab,
Expand Down Expand Up @@ -494,34 +528,26 @@ pub(super) fn solana_deploy(
);

// seeds
let mut seeds = Vec::new();
let mut declared_bump = None;
let mut seeds = func
.annotations
.seeds
.iter()
.map(|seed| expression(&seed.1, cfg, contract_no, None, ns, vartab, opt))
.collect::<Vec<Expression>>();

for note in &func.annotations {
match note {
ConstructorAnnotation::Seed(seed) => {
seeds.push(expression(seed, cfg, contract_no, None, ns, vartab, opt));
}
ConstructorAnnotation::Bump(bump) => {
let expr = ast::Expression::Cast {
loc: Loc::Codegen,
to: Type::Slice(Type::Bytes(1).into()),
expr: ast::Expression::BytesCast {
loc: Loc::Codegen,
to: Type::DynamicBytes,
from: Type::Bytes(1),
expr: bump.clone().into(),
}
.into(),
};
declared_bump = Some(expr);
if let Some((_, bump)) = &func.annotations.bump {
let expr = ast::Expression::Cast {
loc: Loc::Codegen,
to: Type::Slice(Type::Bytes(1).into()),
expr: ast::Expression::BytesCast {
loc: Loc::Codegen,
to: Type::DynamicBytes,
from: Type::Bytes(1),
expr: bump.clone().into(),
}
_ => (),
}
}

if let Some(bump) = declared_bump {
seeds.push(expression(&bump, cfg, contract_no, None, ns, vartab, opt));
.into(),
};
seeds.push(expression(&expr, cfg, contract_no, None, ns, vartab, opt));
}

let seeds = if !seeds.is_empty() {
Expand Down
39 changes: 13 additions & 26 deletions src/sema/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ pub struct Function {
/// For overloaded functions this is the mangled (unique) name.
pub mangled_name: String,
/// Solana constructors may have seeds specified using @seed tags
pub annotations: Vec<ConstructorAnnotation>,
pub annotations: ConstructorAnnotations,
/// Which contracts should we use the mangled name in?
pub mangled_name_contracts: HashSet<usize>,
/// This indexmap stores the accounts this functions needs to be called on Solana
Expand All @@ -366,26 +366,17 @@ pub struct SolanaAccount {
pub generated: bool,
}

#[derive(Debug)]
pub enum ConstructorAnnotation {
Seed(Expression),
Payer(pt::Loc, String),
Space(Expression),
Bump(Expression),
#[derive(Debug, Default)]
pub struct ConstructorAnnotations {
// (annotation location, annotation expression)
pub seeds: Vec<(pt::Loc, Expression)>,
pub space: Option<(pt::Loc, Expression)>,
pub bump: Option<(pt::Loc, Expression)>,
// (annotation location, account name)
pub payer: Option<(pt::Loc, String)>,
}

impl CodeLocation for ConstructorAnnotation {
fn loc(&self) -> pt::Loc {
match self {
ConstructorAnnotation::Seed(expr)
| ConstructorAnnotation::Space(expr)
| ConstructorAnnotation::Bump(expr) => expr.loc(),
ConstructorAnnotation::Payer(loc, _) => *loc,
}
}
}

/// This trait provides a single interface for fetching paramenters, returns and the symbol table
/// This trait provides a single interface for fetching parameters, returns and the symbol table
/// for both yul and solidity functions
pub trait FunctionAttributes {
fn get_symbol_table(&self) -> &Symtable;
Expand Down Expand Up @@ -464,7 +455,7 @@ impl Function {
symtable: Symtable::new(),
emits_events: Vec::new(),
mangled_name,
annotations: Vec::new(),
annotations: ConstructorAnnotations::default(),
mangled_name_contracts: HashSet::new(),
solana_accounts: IndexMap::new().into(),
}
Expand Down Expand Up @@ -509,16 +500,12 @@ impl Function {

/// Does this function have an @payer annotation?
pub fn has_payer_annotation(&self) -> bool {
self.annotations
.iter()
.any(|note| matches!(note, ConstructorAnnotation::Payer(..)))
self.annotations.payer.is_some()
}

/// Does this function have an @seed annotation?
pub fn has_seed_annotation(&self) -> bool {
self.annotations
.iter()
.any(|note| matches!(note, ConstructorAnnotation::Seed(..)))
!self.annotations.seeds.is_empty()
}

/// Does this function have the pure state
Expand Down
Loading

0 comments on commit 614d413

Please sign in to comment.