Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) #3476

pventuzelo · 2023-09-11T15:49:09Z

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS)

We (@FuzzingLabs) found a “len out of range” Panic issue while Unmarshaling Scale.

Expected behavior

The function should check the length of the slice.

Environment

Ubuntu 22.04.1
go version go1.21.0 linux/amd64

Steps to reproduce

This test crashes.

func TestScaleUnmarshal(t *testing.T) {
  block := NewBlock(*NewEmptyHeader(), Body{})
  err := scale.Unmarshal([]byte{48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 4, 48, 48, 48, 48, 19, 48, 48, 48, 48, 48, 48, 48, 48}, &block)
  if err != nil {
    t.Fatal(err)
  }
}

Root cause

The decodeBytes function tries to call make on the length read from the given bytes. By doing so it reads a very big value and fails to create a slice.

gossamer/pkg/scale/decode.go

Line 610 in d5c2c7f

b := make([]byte, length)

panic: runtime error: makeslice: len out of range [recovered]
	panic: runtime error: makeslice: len out of range

Detailed behavior

panic: runtime error: makeslice: len out of range [recovered]
	panic: runtime error: makeslice: len out of range

goroutine 8 [running]:
testing.tRunner.func1.2({0x954620, 0xab3230})
	/snap/go/10319/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
	/snap/go/10319/src/testing/testing.go:1548 +0x397
panic({0x954620?, 0xab3230?})
	/snap/go/10319/src/runtime/panic.go:914 +0x21f
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeBytes(0xc00037c370, {0x91f1c0?, 0xc00007d0c8?, 0x20?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:615 +0xca
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x91f1c0?, {0x91f1c0?, 0xc00007d0c8?, 0x4d3025?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:126 +0x2a5
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeStruct(0x991b40?, {0x991b40?, 0xc00007d040?, 0xac0e48?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:490 +0x357
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x9577a0?, {0x991b40?, 0xc00007d040?, 0x991b40?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:151 +0x63e
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeVaryingDataType(0xc00037c370, {0x9781c0?, 0xc0000136b0?, 0x18?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:394 +0x21b
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x9646c0?, {0x9781c0?, 0xc0000136b0?, 0x9781c0?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:134 +0x105
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeVaryingDataTypeSlice(0x9841a0?, {0x9841a0?, 0xc0000004e8?, 0x2?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:339 +0x29f
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x9841a0?, {0x9841a0?, 0xc0000004e8?, 0x9841a0?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:136 +0x20d
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeStruct(0x9b1a20?, {0x9b1a20?, 0xc0002ccdd0?, 0xac0e48?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:490 +0x357
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x9b1a20?, {0x9b1a20?, 0xc0002ccdd0?, 0x9b1a20?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:151 +0x63e
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).decodeStruct(0x974de0?, {0x974de0?, 0xc0002cca90?, 0xac0e48?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:490 +0x357
github.com/ChainSafe/gossamer/pkg/scale.(*decodeState).unmarshal(0x985b60?, {0x974de0?, 0xc0002cca90?, 0x1?})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:151 +0x63e
github.com/ChainSafe/gossamer/pkg/scale.Unmarshal({0xc000032620, 0x70, 0x70}, {0x985b60, 0xc0002cca90})
	/home/tanguy/Documents/polkadot/gossamer/pkg/scale/decode.go:74 +0x1ed
github.com/ChainSafe/gossamer/dot/types.FuzzUnmarshalBlock.func1(0xc0001f5520, {0xc000032620, 0x70, 0x70})
	/home/tanguy/Documents/polkadot/gossamer/dot/types/block_test.go:141 +0x27c
reflect.Value.call({0x932600?, 0xa29dd8?, 0x13?}, {0x9e247e, 0x4}, {0xc0002caae0, 0x2, 0x2?})
	/snap/go/10319/src/reflect/value.go:596 +0xce7
reflect.Value.Call({0x932600?, 0xa29dd8?, 0xe1cbe0?}, {0xc0002caae0?, 0x9e0fa0?, 0x527e8d?})
	/snap/go/10319/src/reflect/value.go:380 +0xb9
testing.(*F).Fuzz.func1.1(0x0?)
	/snap/go/10319/src/testing/fuzz.go:335 +0x347
testing.tRunner(0xc0001f5520, 0xc0000b4360)
	/snap/go/10319/src/testing/testing.go:1595 +0xff
created by testing.(*F).Fuzz.func1 in goroutine 7
	/snap/go/10319/src/testing/fuzz.go:322 +0x597
exit status 2

The text was updated successfully, but these errors were encountered:

pventuzelo · 2024-01-26T09:46:47Z

Hi, do you have any update regarding the fix of this bug?

P1sar · 2024-01-29T12:34:00Z

Hey @pventuzelo unfortunately we have not looked at this yet, but will prioritise this and will return with some updates asap.
Thank you for submitting the problem!

kishansagathiya · 2024-01-30T15:06:41Z

I think the right way to solve this is to pick a max memory size which we are comfortable to decode and through an error on any request asking to decode bytes more than that memory.
Inside the project, the highest number of bytes we would want to decode does not exceed 16mb (that's MaxBlockResponseSize). I think 16mb would be a good maxMemory.

@timwu20 wdyt?

timwu20 · 2024-01-30T18:14:43Z

I think the right way to solve this is to pick a max memory size which we are comfortable to decode and through an error on any request asking to decode bytes more than that memory. Inside the project, the highest number of bytes we would want to decode does not exceed 16mb (that's MaxBlockResponseSize). I think 16mb would be a good maxMemory.

@timwu20 wdyt?

I think we should check the parity codec and see what the maximum size is. I believe they instituted a maximum size.

kishansagathiya · 2024-01-31T11:01:35Z

In parity scale codec, a trait called MaxEncodedLen gets used. Using which different types could have different maximum encoded length. We could check against this length while decoding.

We could get idea of those max encoded len values from this test file.
https://github.com/paritytech/parity-scale-codec/blob/8c3cc7f2311c8ea7522f4edc21d9cf1f7b661519/tests/max_encoded_len.rs

kishansagathiya · 2024-02-01T09:24:02Z

I think we should check the parity codec and see what the maximum size is. I believe they instituted a maximum size.

I have been searching through parity-scale-codec and to figure out this maximum size. Tried to go through the macros as well amongst other things to figure this out. But couldn't find this max value.

I then tried to decode these same bytes using parity-scale-codec to see if it fails gracefully or panics. It panicked. So, that makes me believe there is not max value.

I basically added these lines inside the test ensure_format_is_unchanged

		let encoded = vec![48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 4, 48, 48, 48, 48, 19, 48, 48, 48, 48, 48, 48, 48, 48];

		let decoded: Block<Header<u64, BlakeTwo256>, Vec<u8>> = Decode::decode(&mut &encoded[..]).unwrap();

which resulted in below panic

running 1 test
thread 'generic::header::tests::ensure_format_is_unchanged' panicked at 'called `Result::unwrap()` on an `Err` value: Error { cause: Some(Error { cause: Some(Error { cause: Some(Error { cause: None, desc: "out of range decoding Compact<u32>" }), desc: "Could not decode `Digest::logs`" }), desc: "Could not decode `Header::digest`" }), desc: "Could not decode `Block::header`" }', substrate/primitives/runtime/src/generic/header.rs:211:99
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1651:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1076:23
   4: sp_runtime::generic::header::tests::ensure_format_is_unchanged
             at ./src/generic/header.rs:211:59
   5: sp_runtime::generic::header::tests::ensure_format_is_unchanged::{{closure}}
             at ./src/generic/header.rs:208:34
   6: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test generic::header::tests::ensure_format_is_unchanged ... FAILED

failures:

failures:
    generic::header::tests::ensure_format_is_unchanged

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 56 filtered out; finished in 0.36s

error: test failed, to rerun pass `-p sp-runtime --lib`

 *  The terminal process "cargo 'test', '--package', 'sp-runtime', '--lib', '--', 'generic::header::tests::ensure_format_is_unchanged', '--exact', '--nocapture'" terminated with exit code: 101. 
 *  Terminal will be reused by tasks, press any key to close it.

kishansagathiya · 2024-02-01T10:45:17Z

@timwu20 @P1sar
Based on #3476 (comment)

Should I create an issue to have max encoded length for each types? We could add them to each decode funcs of each types and make them safer.

timwu20 · 2024-02-01T17:42:04Z

I think we should check the parity codec and see what the maximum size is. I believe they instituted a maximum size.

I have been searching through parity-scale-codec and to figure out this maximum size. Tried to go through the macros as well amongst other things to figure this out. But couldn't find this max value.

I then tried to decode these same bytes using parity-scale-codec to see if it fails gracefully or panics. It panicked. So, that makes me believe there is not max value.

I basically added these lines inside the test ensure_format_is_unchanged

		let encoded = vec![48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 48, 4, 48, 48, 48, 48, 19, 48, 48, 48, 48, 48, 48, 48, 48];

		let decoded: Block<Header<u64, BlakeTwo256>, Vec<u8>> = Decode::decode(&mut &encoded[..]).unwrap();

which resulted in below panic

running 1 test
thread 'generic::header::tests::ensure_format_is_unchanged' panicked at 'called `Result::unwrap()` on an `Err` value: Error { cause: Some(Error { cause: Some(Error { cause: Some(Error { cause: None, desc: "out of range decoding Compact<u32>" }), desc: "Could not decode `Digest::logs`" }), desc: "Could not decode `Header::digest`" }), desc: "Could not decode `Block::header`" }', substrate/primitives/runtime/src/generic/header.rs:211:99
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
   2: core::result::unwrap_failed
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1651:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1076:23
   4: sp_runtime::generic::header::tests::ensure_format_is_unchanged
             at ./src/generic/header.rs:211:59
   5: sp_runtime::generic::header::tests::ensure_format_is_unchanged::{{closure}}
             at ./src/generic/header.rs:208:34
   6: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test generic::header::tests::ensure_format_is_unchanged ... FAILED

failures:

failures:
    generic::header::tests::ensure_format_is_unchanged

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 56 filtered out; finished in 0.36s

error: test failed, to rerun pass `-p sp-runtime --lib`

 *  The terminal process "cargo 'test', '--package', 'sp-runtime', '--lib', '--', 'generic::header::tests::ensure_format_is_unchanged', '--exact', '--nocapture'" terminated with exit code: 101. 
 *  Terminal will be reused by tasks, press any key to close it.

Well it's panicking cause you called unwrap() when an error was returned from the result. I don't think we can assume that the scale codec panics based on this. We should try to replicate the error returned which in this case is out of range decoding Compact<u32>.

github-actions · 2024-03-01T16:45:23Z

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

pventuzelo added the Type: Bug label Sep 11, 2023

pventuzelo changed the title ~~Gossamer Issue 1 - Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS)~~ Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) Sep 11, 2023

P1sar removed the Type: Bug label Jan 15, 2024

P1sar added T-bug this issue covers unexpected and/or wrong behaviour. S-scale issues related to the pkg/scale package. labels Jan 29, 2024

P1sar added the P-high this should be addressed ASAP. label Jan 29, 2024

P1sar assigned kishansagathiya Jan 29, 2024

kishansagathiya mentioned this issue Feb 1, 2024

fix: add a limit of number of bytes while scale decoding a slice #3733

Merged

P1sar added this to the Release v0.9.0 milestone Feb 1, 2024

kishansagathiya closed this as completed in #3733 Feb 12, 2024

github-actions bot added the released label Mar 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) #3476

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) #3476

pventuzelo commented Sep 11, 2023 •

edited

Loading

pventuzelo commented Jan 26, 2024

P1sar commented Jan 29, 2024

kishansagathiya commented Jan 30, 2024 •

edited

Loading

timwu20 commented Jan 30, 2024

kishansagathiya commented Jan 31, 2024 •

edited

Loading

kishansagathiya commented Feb 1, 2024

kishansagathiya commented Feb 1, 2024

timwu20 commented Feb 1, 2024 •

edited

Loading

github-actions bot commented Mar 1, 2024

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) #3476

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS) #3476

Comments

pventuzelo commented Sep 11, 2023 • edited Loading

Panic “len out of range” while Unmarshaling Scale leading to a denial of service (DoS)

Expected behavior

Environment

Steps to reproduce

Root cause

Detailed behavior

pventuzelo commented Jan 26, 2024

P1sar commented Jan 29, 2024

kishansagathiya commented Jan 30, 2024 • edited Loading

timwu20 commented Jan 30, 2024

kishansagathiya commented Jan 31, 2024 • edited Loading

kishansagathiya commented Feb 1, 2024

kishansagathiya commented Feb 1, 2024

timwu20 commented Feb 1, 2024 • edited Loading

github-actions bot commented Mar 1, 2024

pventuzelo commented Sep 11, 2023 •

edited

Loading

kishansagathiya commented Jan 30, 2024 •

edited

Loading

kishansagathiya commented Jan 31, 2024 •

edited

Loading

timwu20 commented Feb 1, 2024 •

edited

Loading