multiformats / rust-multiaddr

multiaddr implementation in rust
https://crates.io/crates/multiaddr
Other
86 stars 45 forks source link

Add test failing because decapsulate is based on substrings. #18

Open progval opened 7 years ago

progval commented 7 years ago

decapsulate uses a substring matching technique to find the subaddress to be removed. However, there is no safeguard for this substring to be part of content before the subaddress, eg. an IP address.

This PR adds a test that crashes if the vulnerability exist:

---- decapsulate_no_substring stdout ----
        wrote 41
Got an address [48, 49, 101, 48, 58, 58]
IP6, "01e0::"
address [1, 224, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
wrote 480
Got an address [101, 120, 97, 109, 112, 108, 101, 46, 99, 111, 109]
HTTP, "example.com"
address []
remain: ""
wrote 480
remain: ""
Multiaddr { bytes: [0, 41, 1, 224, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 224] }
Multiaddr { bytes: [1, 224] }
Multiaddr { bytes: [0, 41] }
code 41
taking size 128
[]
code 41
Error(Position(Many1, [0, 41]))
thread 'decapsulate_no_substring' panicked at 'Failed to parse internal bytes, possible corruption', src/parser.rs:156
stack backtrace:
   1:     0x55ec5b385e8f - std::sys::backtrace::tracing::imp::write::h3800f45f421043b8
   2:     0x55ec5b38960b - std::panicking::default_hook::_$u7b$$u7b$closure$u7d$$u7d$::h0ef6c8db532f55dc
   3:     0x55ec5b38920c - std::panicking::default_hook::hf3839060ccbb8764
   4:     0x55ec5b37bccd - std::panicking::rust_panic_with_hook::h5dd7da6bb3d06020
   5:     0x55ec5b335217 - std::panicking::begin_panic::h4c14e704ed94392f
                        at src/libstd/panicking.rs:328
   6:     0x55ec5b330aa0 - multiaddr::parser::address_from_bytes::hd604e7e6993c1768
                        at /home/dev-ipfs/rust-multiaddr/<std macros>:3
   7:     0x55ec5b3304f7 - _<Multiaddr as std..string..ToString>::to_string::h2d7cf3ad58ec2043
                        at src/lib.rs:53
   8:     0x55ec5b3318aa - multiaddr::decapsulate_no_substring::hfeaba95b02e8c4ae
                        at src/lib.rs:203
   9:     0x55ec5b360796 - _<F as std..boxed..FnBox<A>>::call_box::h6f41cb4b13d7cb46
  10:     0x55ec5b362ec7 - std::panicking::try::call::h3736ed37d7dc244b
  11:     0x55ec5b391f6b - __rust_try
  12:     0x55ec5b391f0e - __rust_maybe_catch_panic
  13:     0x55ec5b3632ee - _<F as std..boxed..FnBox<A>>::call_box::h70b0cca3005960f6
  14:     0x55ec5b387d94 - std::sys::thread::Thread::new::thread_start::h9e5bde00f3b3e2e2
  15:     0x7f1c3d18e443 - start_thread
  16:     0x7f1c3ccb720c - clone
  17:                0x0 - <unknown>

I am not sending a fix right now, because I think the best way to fix that would be to change the internal structure of Multiaddr. See issue #19