Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack overflow issue in scalecodec while decoding inputs #425

Closed
mmostafas opened this issue Apr 18, 2023 · 6 comments · Fixed by #426
Closed

Stack overflow issue in scalecodec while decoding inputs #425

mmostafas opened this issue Apr 18, 2023 · 6 comments · Fixed by #426
Assignees

Comments

@mmostafas
Copy link

There is an stack overflow issue in the parity-scale-codec crate that crashes while decoding inputs. We ran into this issue when decoding an XCM call but we believe it can be triggered from other code paths as well.

Here is a tests that triggers this bug:

#[test]
fn decode_instruction_segfault() {
	use parity_scale_codec::DecodeLimit;
	let mut decoded = VersionedXcm::V3(v3::Xcm::<()>::new());
	let encoded = hex_literal::hex!("03f800000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515151515");

	decoded  = DecodeLimit::decode_with_depth_limit(20, &mut encoded.as_slice()).unwrap();
}

Please note that the depth limit of 20 is used here which is relatively low.

There is also a similar issue submitted here.

@koute
Copy link
Contributor

koute commented Apr 20, 2023

@mmostafas Could you check whether this PR fixes your issue?

#426

@mmostafas
Copy link
Author

Thank you @koute,

Unfortunately #426 did not fix our issues. If you want to reproduce the bug, you can do so by adding the provided test above to the unit tests for the XCM.

Please let me know if you need more info.

@koute
Copy link
Contributor

koute commented Apr 24, 2023

@mmostafas Thanks for the explanation on how to reproduce it!

I confirm I can reproduce the issue. I'll fix it.

@koute koute self-assigned this Apr 24, 2023
@koute
Copy link
Contributor

koute commented Apr 24, 2023

I've found why this happens; it's due to this upstream rustc bug:

rust-lang/rust#34283

The Instruction enum is 1472 bytes big and has a ton of variants. When it is decoded there's a match over every variant, and for every variant it seems that extra stack space is consumed proportional to how big the enum is. Then nest it a few times and you get a stack overflow.

@koute
Copy link
Contributor

koute commented Apr 24, 2023

Okay, I think I fixed it this time!

@mmostafas
Copy link
Author

I can confirm this fixes our issues. Thanks @koute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants