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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

progval
Copy link
Contributor

@progval progval commented Sep 18, 2016

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

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 this pull request may close these issues.

1 participant