Skip to content

Commit

Permalink
Merge pull request #591 from stacks-network/fix/from-consensus-buff-m…
Browse files Browse the repository at this point in the history 
…emory-oob

OOB memory access on `from-consensus-buff?`
  • Loading branch information
@BowTiedWoo
BowTiedWoo authored Jan 7, 2025
2 parents 41079f3 + 2cef72a commit 22d35e4
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 35 deletions.
121 changes: 87 additions & 34 deletions clar2wasm/src/deserialize.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ impl WasmGenerator {
builder: &mut InstrSeqBuilder,
memory: MemoryId,
offset_local: LocalId,
offset_result: LocalId,
end_local: LocalId,
) -> Result<(), GeneratorError> {
// Create a block for the body of this operation, so that we can
Expand Down Expand Up @@ -195,22 +196,22 @@ impl WasmGenerator {
then.i32_const(1);

// Allocate space for the principal on the call stack
let result_offset = self.module.locals.add(ValType::I32);
then.global_get(self.stack_pointer).local_tee(result_offset);
let principal_offset = self.module.locals.add(ValType::I32);
then.local_get(offset_result).local_tee(principal_offset);
then.i32_const(STANDARD_PRINCIPAL_BYTES as i32)
.binop(BinaryOp::I32Add)
.global_set(self.stack_pointer);
.local_set(offset_result);

// Copy the principal to the destination
then.local_get(result_offset)
then.local_get(principal_offset)
.local_get(offset_local)
.i32_const(1)
.binop(BinaryOp::I32Add)
.i32_const(PRINCIPAL_BYTES as i32)
.memory_copy(memory, memory);

// Write the contract name length (0)
then.local_get(result_offset).i32_const(0).store(
then.local_get(principal_offset).i32_const(0).store(
memory,
StoreKind::I32_8 { atomic: false },
MemArg {
Expand All @@ -227,7 +228,7 @@ impl WasmGenerator {
.local_set(offset_local);

// Push the offset and length onto the stack
then.local_get(result_offset)
then.local_get(principal_offset)
.i32_const(STANDARD_PRINCIPAL_BYTES as i32);

// Break out of the block
Expand Down Expand Up @@ -431,6 +432,7 @@ impl WasmGenerator {
memory: MemoryId,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
value_ty: &TypeSignature,
) -> Result<(), GeneratorError> {
// Create a block for the body of this operation, so that we can
Expand Down Expand Up @@ -513,11 +515,17 @@ impl WasmGenerator {
.local_set(offset_local);

// Deserialize the inner value
self.deserialize_from_memory(&mut some_block, offset_local, end_local, value_ty)?;
self.deserialize_from_memory(
&mut some_block,
offset_local,
end_local,
offset_result,
value_ty,
)?;

// Check if the deserialization failed:
// - Store the value in locals
// - Check the inidicator now on top of the stack
// - Check the indicator now on top of the stack
let inner_locals = self.save_to_locals(&mut some_block, value_ty, true);
some_block.unop(UnaryOp::I32Eqz).if_else(
None,
Expand Down Expand Up @@ -573,12 +581,14 @@ impl WasmGenerator {
/// | 0x07 | serialized ok value |
/// Err:
/// | 0x08 | serialized err value |
#[allow(clippy::too_many_arguments)]
fn deserialize_response(
&mut self,
builder: &mut InstrSeqBuilder,
memory: MemoryId,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
ok_ty: &TypeSignature,
err_ty: &TypeSignature,
) -> Result<(), GeneratorError> {
Expand Down Expand Up @@ -638,7 +648,7 @@ impl WasmGenerator {
.local_set(offset_local);

// Deserialize the inner value
self.deserialize_from_memory(&mut ok_block, offset_local, end_local, ok_ty)?;
self.deserialize_from_memory(&mut ok_block, offset_local, end_local, offset_result, ok_ty)?;

// Check if the deserialization failed:
// - Store the value in locals
Expand Down Expand Up @@ -698,7 +708,13 @@ impl WasmGenerator {
.local_set(offset_local);

// Deserialize the inner value
self.deserialize_from_memory(&mut err_block, offset_local, end_local, err_ty)?;
self.deserialize_from_memory(
&mut err_block,
offset_local,
end_local,
offset_result,
err_ty,
)?;

// Check if the deserialization failed:
// - Store the value in locals
Expand Down Expand Up @@ -759,6 +775,7 @@ impl WasmGenerator {
memory: MemoryId,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
list_ty: &ListTypeData,
) -> Result<(), GeneratorError> {
// Create a block for the body of this operation, so that we can
Expand Down Expand Up @@ -840,18 +857,18 @@ impl WasmGenerator {
// Allocate space for the list on the call stack
let element_ty = list_ty.get_list_item_type();
let result = self.module.locals.add(ValType::I32);
let result_offset = self.module.locals.add(ValType::I32);
let element_offset = self.module.locals.add(ValType::I32);
let element_size = get_type_size(element_ty);
block
.global_get(self.stack_pointer)
.local_get(offset_result)
.local_tee(result)
.local_tee(result_offset);
.local_tee(element_offset);
block
.local_get(length)
.i32_const(element_size)
.binop(BinaryOp::I32Mul)
.binop(BinaryOp::I32Add)
.global_set(self.stack_pointer);
.local_set(offset_result);

// Update the offset to point to the first element
block
Expand Down Expand Up @@ -893,7 +910,13 @@ impl WasmGenerator {

// Deserialize the element. Note, this will update the offset to point
// to the next element.
self.deserialize_from_memory(&mut loop_block, offset_local, end_local, element_ty)?;
self.deserialize_from_memory(
&mut loop_block,
offset_local,
end_local,
offset_result,
element_ty,
)?;

// Check if the deserialization failed:
// - Store the value in locals
Expand All @@ -917,14 +940,14 @@ impl WasmGenerator {
for local in inner_locals {
loop_block.local_get(local);
}
let bytes_written = self.write_to_memory(&mut loop_block, result_offset, 0, element_ty)?;
let bytes_written = self.write_to_memory(&mut loop_block, element_offset, 0, element_ty)?;

// Increment the result offset by the number of bytes written
loop_block
.local_get(result_offset)
.local_get(element_offset)
.i32_const(bytes_written as i32)
.binop(BinaryOp::I32Add)
.local_set(result_offset);
.local_set(element_offset);

// Increment the index by 1
loop_block
Expand Down Expand Up @@ -960,6 +983,7 @@ impl WasmGenerator {
memory: MemoryId,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
tuple_ty: &TupleTypeSignature,
) -> Result<(), GeneratorError> {
// We need to be able to parse the keys coming in a random order, only one occurence of each key.
Expand Down Expand Up @@ -1214,6 +1238,7 @@ impl WasmGenerator {
&mut case_block,
offset_local,
end_local,
offset_result,
field_ty,
)?;
for &l in field_locals.iter().rev() {
Expand Down Expand Up @@ -1629,6 +1654,7 @@ impl WasmGenerator {
memory: MemoryId,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
string_utf8_ty: &TypeSignature,
) -> Result<(), GeneratorError> {
let max_len: u32 = match string_utf8_ty {
Expand Down Expand Up @@ -1694,9 +1720,7 @@ impl WasmGenerator {
then.if_else(
return_type,
|then| {
let (offset_result, _len) =
self.create_call_stack_local(then, string_utf8_ty, false, true);

// convert utf8 to string-utf8
then.local_get(offset_local)
.local_get(string_length)
.local_get(offset_result)
Expand All @@ -1708,6 +1732,12 @@ impl WasmGenerator {
.local_get(string_length)
.binop(BinaryOp::I32Add)
.local_set(offset_local);

// move offset-result to the end of the deserialized utf8 string
then.local_get(offset_result)
.i32_const(max_len as i32 * 4)
.binop(BinaryOp::I32Add)
.local_set(offset_result);
},
|else_| {
else_.i32_const(0).i32_const(0).i32_const(0);
Expand Down Expand Up @@ -1735,6 +1765,7 @@ impl WasmGenerator {
builder: &mut InstrSeqBuilder,
offset_local: LocalId,
end_local: LocalId,
offset_result: LocalId,
ty: &TypeSignature,
) -> Result<(), GeneratorError> {
let memory = self.get_memory()?;
Expand All @@ -1745,23 +1776,34 @@ impl WasmGenerator {
self.deserialize_integer(builder, memory, offset_local, end_local, ty == &IntType)
}
PrincipalType | CallableType(_) | TraitReferenceType(_) => {
self.deserialize_principal(builder, memory, offset_local, end_local)
self.deserialize_principal(builder, memory, offset_local, offset_result, end_local)
}
ResponseType(types) => self.deserialize_response(
builder,
memory,
offset_local,
end_local,
offset_result,
&types.0,
&types.1,
),
BoolType => self.deserialize_bool(builder, memory, offset_local, end_local),
OptionalType(value_ty) => {
self.deserialize_optional(builder, memory, offset_local, end_local, value_ty)
}
SequenceType(SequenceSubtype::ListType(list_ty)) => {
self.deserialize_list(builder, memory, offset_local, end_local, list_ty)
}
OptionalType(value_ty) => self.deserialize_optional(
builder,
memory,
offset_local,
end_local,
offset_result,
value_ty,
),
SequenceType(SequenceSubtype::ListType(list_ty)) => self.deserialize_list(
builder,
memory,
offset_local,
end_local,
offset_result,
list_ty,
),
SequenceType(SequenceSubtype::BufferType(type_length)) => self.deserialize_buffer(
builder,
memory,
Expand All @@ -1777,12 +1819,23 @@ impl WasmGenerator {
end_local,
type_length.into(),
),
utf8 @ SequenceType(SequenceSubtype::StringType(StringSubtype::UTF8(_))) => {
self.deserialize_string_utf8(builder, memory, offset_local, end_local, utf8)
}
TupleType(tuple_ty) => {
self.deserialize_tuple(builder, memory, offset_local, end_local, tuple_ty)
}
utf8 @ SequenceType(SequenceSubtype::StringType(StringSubtype::UTF8(_))) => self
.deserialize_string_utf8(
builder,
memory,
offset_local,
end_local,
offset_result,
utf8,
),
TupleType(tuple_ty) => self.deserialize_tuple(
builder,
memory,
offset_local,
end_local,
offset_result,
tuple_ty,
),
NoType => unreachable!("NoType should not be deserialized"),
ListUnionType(_) => unreachable!("ListUnionType should not be deserialized"),
}
Expand Down
5 changes: 4 additions & 1 deletion clar2wasm/src/words/consensus_buff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ impl ComplexWord for FromConsensusBuff {
// Traverse the input buffer, leaving the offset and length on the stack.
generator.traverse_expr(builder, args.get_expr(1)?)?;

// TODO: true, true is too big; see issue: #593
let (offset_result, _len) = generator.create_call_stack_local(builder, &ty, true, true);
let offset = generator.module.locals.add(walrus::ValType::I32);
let end = generator.module.locals.add(walrus::ValType::I32);
builder
Expand All @@ -134,7 +136,8 @@ impl ComplexWord for FromConsensusBuff {
.binop(BinaryOp::I32Add)
.local_set(end);

generator.deserialize_from_memory(builder, offset, end, &value_ty)?;
// Write the deserialized value on the stack at offset_result
generator.deserialize_from_memory(builder, offset, end, offset_result, &value_ty)?;

// If the entire buffer was not consumed, return none.
builder
Expand Down

0 comments on commit 22d35e4

Please sign in to comment.