osrg / rustybgp

BGP implemented in the Rust Programming Language
Apache License 2.0
478 stars 46 forks source link

Panic on injecting routes - range start index 4 out of range for slice of length 0 #251

Open hprem opened 5 months ago

hprem commented 5 months ago

Hello, While trying out rustybgp, I am hitting this panic on injecting routes from the mrt dump http://data.ris.ripe.net/rrc01/2010.01/bview.20100101.0759.gz. Was able to inject ~ 80k routes without any issues, before hitting the issue

thread '<unnamed>' panicked at 'range start index 4 out of range for slice of length 0', daemon/src/packet/bgp.rs:936:26

$ gobgp mrt inject global bview.20100101.0759

Config

$ cat gobgpd.conf
[global.config]
  as = 30656
  router-id = "10.1.1.1"
  #local-address-list = ["0.0.0.0"]

[[neighbors]]
  [neighbors.config]
    neighbor-address = "10.100.1.3"
    peer-as = 30656
  [neighbors.transport.config]
    local-address = "10.100.1.101"
  [[neighbors.afi-safis]]
    [neighbors.afi-safis.config]
    afi-safi-name = "ipv4-unicast"
  [[neighbors.afi-safis]]
    [neighbors.afi-safis.config]
    afi-safi-name = "ipv6-unicast"

stack backtrace

   0:     0x55ceeeeba791 - std::backtrace_rs::backtrace::libunwind::trace::h6aeaf83abc038fe6
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:     0x55ceeeeba791 - std::backtrace_rs::backtrace::trace_unsynchronized::h4f9875212db0ad97
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:     0x55ceeeeba791 - std::sys_common::backtrace::_print_fmt::h3f820027e9c39d3b
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:65:5
   3:     0x55ceeeeba791 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hded4932df41373b3
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:44:22
   4:     0x55ceeeee172f - core::fmt::rt::Argument::fmt::hc8ead7746b2406d6
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/fmt/rt.rs:138:9
   5:     0x55ceeeee172f - core::fmt::write::hb1cb56105a082ad9
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/fmt/mod.rs:1094:21
   6:     0x55ceeeeb79e1 - std::io::Write::write_fmt::h797fda7085c97e57
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/io/mod.rs:1713:15
   7:     0x55ceeeeba5a5 - std::sys_common::backtrace::_print::h492d3c92d7400346
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:47:5
   8:     0x55ceeeeba5a5 - std::sys_common::backtrace::print::hf74aa2eef05af215
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:34:9
   9:     0x55ceeeebba47 - std::panicking::default_hook::{{closure}}::h8cad394227ea3de8
  10:     0x55ceeeebb834 - std::panicking::default_hook::h249cc184fec99a8a
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:288:9
  11:     0x55ceeeebbefc - std::panicking::rust_panic_with_hook::h82ebcd5d5ed2fad4
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:705:13
  12:     0x55ceeeebbdf7 - std::panicking::begin_panic_handler::{{closure}}::h810bed8ecbe66f1a
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:597:13
  13:     0x55ceeeebabc6 - std::sys_common::backtrace::__rust_end_short_backtrace::h1410008071796261
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:151:18
  14:     0x55ceeeebbb42 - rust_begin_unwind
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
  15:     0x55ceee62cdc3 - core::panicking::panic_fmt::ha0a42a25e0cf258d
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
  16:     0x55ceee62d2e2 - core::slice::index::slice_start_index_len_fail_rt::h647e9cdc9e93900b
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/slice/index.rs:52:5
  17:     0x55ceee62d2e2 - core::slice::index::slice_start_index_len_fail::hc38d63f1acbecfc7
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/slice/index.rs:40:9
  18:     0x55ceee945ab1 - rustybgpd::packet::bgp::Attribute::export::h172af49e4b25b088
  19:     0x55ceee9493d3 - rustybgpd::packet::bgp::Codec::do_encode::h577169166429baba
  20:     0x55ceee94bfa4 - <rustybgpd::packet::bgp::Codec as tokio_util::codec::encoder::Encoder<&rustybgpd::packet::bgp::Message>>::encode::h9c17de7e7ff012f8
  21:     0x55ceeeb93974 - rustybgpd::event::Handler::run::{{closure}}::hc26426cdc00e7491
  22:     0x55ceeeb8be9f - rustybgpd::event::main::{{closure}}::{{closure}}::{{closure}}::h1fda0070682e859b
  23:     0x55ceeeb62be1 - tokio::loom::std::unsafe_cell::UnsafeCell<T>::with_mut::h744f4f287deaa0d3
  24:     0x55ceeea6b2ab - tokio::runtime::task::core::Core<T,S>::poll::he010c5b0a6de8ba5
  25:     0x55ceeebafef1 - tokio::runtime::task::harness::Harness<T,S>::poll::hdd671d791c18c2f0
  26:     0x55ceeea1fee4 - tokio::runtime::scheduler::current_thread::Context::run_task::h2a7232efd0bb4d83
  27:     0x55ceee9ed3a3 - tokio::macros::scoped_tls::ScopedKey<T>::set::h443ef471fb33d8c8
  28:     0x55ceeea1ce6a - tokio::runtime::runtime::Runtime::block_on::h7af0e7b1aaaf9537
  29:     0x55ceee9d0ed5 - std::sys_common::backtrace::__rust_begin_short_backtrace::hca35e829d65151d2
  30:     0x55ceeec57786 - core::ops::function::FnOnce::call_once{{vtable.shim}}::h4e98dfb2629b1e41
  31:     0x55ceeeebe765 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h9adfc2ae43657457
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/alloc/src/boxed.rs:1985:9
  32:     0x55ceeeebe765 - <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once::h14fefbfa7b574396
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/alloc/src/boxed.rs:1985:9
  33:     0x55ceeeebe765 - std::sys::unix::thread::Thread::new::thread_start::ha211bb47f6f5cedc
                               at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys/unix/thread.rs:108:17
  34:     0x7fb4f0c97ada - <unknown>
  35:     0x7fb4f0d2847c - <unknown>
  36:                0x0 - <unknown>
rickpayne commented 4 months ago

I had a quick look at this. The failure is in prepending an ASPath to a route that has no existing AS_SET or AS_SEQUENCE. The code checks the length, but doesn't deal with that lack of length in the second part:

        let data = if len != 0 && buf[0] == Attribute::AS_PATH_TYPE_SEQ && buf[1] < 255 {
            let mut new_buf = Vec::with_capacity(len as usize + 4);
            new_buf.put_u8(buf[0]);
            new_buf.put_u8(buf[1] + 1);
            new_buf.put_u32(as_number);
            new_buf.put(&buf[2..]);
            AttributeData::Bin(new_buf)
        } else {
            let mut new_buf = Vec::with_capacity(len as usize + 6);
            new_buf.put_u8(Attribute::AS_PATH_TYPE_SEQ);
            new_buf.put_u8(1);
            new_buf.put_u32(as_number);
            new_buf.put(&buf[4..]);
            AttributeData::Bin(new_buf)
        };

Looking in the bview file, I see the following. Note the prefix at line 81357 has no AS Path.

81354,84.205.0.0/19 3356 8928 24724 31608
81355,84.205.32.0/19 8419 6461 1299 3307
81356,84.205.80.0/24 6067 16150 1103 1125 196613 12654
81357,84.205.81.0/24
81358,84.205.83.0/24 6067 5413 12654
81359,84.205.84.0/24 6067 16150 8928 513 513 513 513 12654
81360,84.205.85.0/24 6067 13237 12654

So we don't take the first path because len is zero, but then there's an assumption data is there. I think the 'len != 0' check is to protect the AS_PATH_TYPE_SEQ check.