Skip to content

Commit 8d158cc

Browse files
authored
[cherry pick] Cherry pick tests for security fix (#10838)
## Description Copying move-language/move#1034 Thanks to [Zellic](https://www.zellic.io/) and @fcremo for the find and fix! ## Test Plan - It is tests --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes
1 parent cc576a3 commit 8d158cc

File tree

6 files changed

+11112
-6
lines changed

6 files changed

+11112
-6
lines changed

move-binary-format/src/unit_tests/binary_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ fn test_max_number_of_bytecode() {
2020
for _ in 0..u16::MAX - 1 {
2121
nops.push(Bytecode::Nop);
2222
}
23-
nops.push(Bytecode::Ret);
23+
nops.push(Bytecode::Branch(0));
2424

2525
let result = Bytecode::get_successors(u16::MAX - 1, &nops);
26-
assert!(result.is_empty());
26+
assert_eq!(result, vec![0]);
2727
}
2828

2929
proptest! {

move-bytecode-verifier/src/regression_tests/reference_analysis.rs

Lines changed: 117 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,15 @@
33

44
use move_binary_format::{
55
file_format::{
6-
empty_module, AbilitySet, AddressIdentifierIndex, Bytecode::*, CodeUnit, Constant,
7-
FieldDefinition, FunctionDefinition, FunctionHandle, FunctionHandleIndex, IdentifierIndex,
8-
ModuleHandle, ModuleHandleIndex, Signature, SignatureIndex, SignatureToken::*,
6+
empty_module, AbilitySet, AddressIdentifierIndex,
7+
Bytecode::{self, *},
8+
CodeUnit, Constant, FieldDefinition, FunctionDefinition, FunctionHandle,
9+
FunctionHandleIndex, IdentifierIndex, ModuleHandle, ModuleHandleIndex, Signature,
10+
SignatureIndex,
11+
SignatureToken::{self, *},
912
StructDefinition, StructDefinitionIndex, StructFieldInformation, StructHandle,
10-
StructHandleIndex, TypeSignature, Visibility, Visibility::*,
13+
StructHandleIndex, TypeSignature, Visibility,
14+
Visibility::*,
1115
},
1216
CompiledModule,
1317
};
@@ -16,6 +20,8 @@ use move_core_types::{
1620
};
1721
use std::str::FromStr;
1822

23+
use crate::VerifierConfig;
24+
1925
#[test]
2026
fn unbalanced_stack_crash() {
2127
let mut module = empty_module();
@@ -208,3 +214,110 @@ fn borrow_graph() {
208214
let res = crate::verify_module(&module);
209215
assert!(res.is_ok());
210216
}
217+
218+
#[test]
219+
fn indirect_code() {
220+
use Bytecode::*;
221+
let v = 0;
222+
let v_ref = 1;
223+
let x = 2;
224+
let x_ref = 3;
225+
let vsig = SignatureIndex(2);
226+
let next = 16;
227+
let mut code = vec![
228+
// x = 10; x_ref = &mut x;
229+
LdU64(10),
230+
StLoc(x),
231+
MutBorrowLoc(x),
232+
StLoc(x_ref),
233+
// v = vector[100, 1000]; v_ref = &mut v
234+
LdU64(100),
235+
LdU64(1000),
236+
VecPack(vsig, 2),
237+
StLoc(v),
238+
MutBorrowLoc(v),
239+
StLoc(v_ref),
240+
// if (*x_ref == 0) return;
241+
CopyLoc(x_ref),
242+
ReadRef,
243+
LdU64(0),
244+
Eq,
245+
BrFalse(next),
246+
Ret,
247+
];
248+
assert_eq!(code.len(), next as usize);
249+
code.extend(vec![
250+
// creates dangling reference on second iteration
251+
// _ = vec_pop_back(x_ref)
252+
CopyLoc(v_ref),
253+
VecPopBack(vsig),
254+
Pop,
255+
// *x_ref = 0
256+
LdU64(0),
257+
CopyLoc(x_ref),
258+
WriteRef,
259+
// x_ref = vec_mut_borrow(v_ref, 0);
260+
CopyLoc(v_ref),
261+
LdU64(0),
262+
VecMutBorrow(vsig),
263+
StLoc(x_ref),
264+
]);
265+
let nops = vec![Nop; (u16::MAX as usize) - code.len() - 1];
266+
code.extend(nops);
267+
code.push(Branch(10));
268+
assert_eq!(code.len(), (u16::MAX as usize));
269+
let module = CompiledModule {
270+
version: 5,
271+
self_module_handle_idx: ModuleHandleIndex(0),
272+
module_handles: vec![ModuleHandle {
273+
address: AddressIdentifierIndex(0),
274+
name: IdentifierIndex(0),
275+
}],
276+
struct_handles: vec![],
277+
function_handles: vec![FunctionHandle {
278+
module: ModuleHandleIndex(0),
279+
name: IdentifierIndex(0),
280+
parameters: SignatureIndex(0),
281+
return_: SignatureIndex(0),
282+
type_parameters: vec![],
283+
}],
284+
field_handles: vec![],
285+
friend_decls: vec![],
286+
struct_def_instantiations: vec![],
287+
function_instantiations: vec![],
288+
field_instantiations: vec![],
289+
signatures: vec![
290+
Signature(vec![]),
291+
Signature(vec![
292+
SignatureToken::Vector(Box::new(SignatureToken::U64)),
293+
SignatureToken::MutableReference(Box::new(SignatureToken::Vector(Box::new(
294+
SignatureToken::U64,
295+
)))),
296+
SignatureToken::U64,
297+
SignatureToken::MutableReference(Box::new(SignatureToken::U64)),
298+
]),
299+
Signature(vec![SignatureToken::U64]),
300+
],
301+
identifiers: vec![Identifier::new("a").unwrap()],
302+
address_identifiers: vec![AccountAddress::ONE],
303+
constant_pool: vec![],
304+
metadata: vec![],
305+
struct_defs: vec![],
306+
function_defs: vec![FunctionDefinition {
307+
function: FunctionHandleIndex(0),
308+
visibility: Visibility::Public,
309+
is_entry: false,
310+
acquires_global_resources: vec![],
311+
code: Some(CodeUnit {
312+
locals: SignatureIndex(1),
313+
code,
314+
}),
315+
}],
316+
};
317+
318+
let res = crate::verify_module_with_config(&VerifierConfig::unbounded(), &module).unwrap_err();
319+
assert_eq!(
320+
res.major_status(),
321+
StatusCode::VEC_UPDATE_EXISTS_MUTABLE_BORROW_ERROR
322+
);
323+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
processed 2 tasks
2+
3+
task 1 'run'. lines 17-5495:
4+
Error: Script execution failed with VMError: {
5+
major_status: STLOC_UNSAFE_TO_DESTROY_ERROR,
6+
sub_status: None,
7+
location: script,
8+
indices: [],
9+
offsets: [(FunctionDefinitionIndex(0), 65530)],
10+
}

0 commit comments

Comments
 (0)