search

Johni0702 / mumble-web-proxy

Mumble to WebSocket+WebRTC proxy for use with mumble-web
70 stars 25 forks source link

how to crash mumble-web-proxy over the network #2

Closed streaps closed 5 years ago

streaps commented 5 years ago

It's very easy to crash mumble-web-proxy over the network:

nc localhost 64737 STRG-C

It immediately crashes when netcat closes the connection.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Io(Custom { kind: Other, error: StringError("bytes remaining on stream") })', libcore/result.rs:1009:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
   1: std::sys_common::backtrace::print
   2: std::panicking::default_hook::{{closure}}
   3: std::panicking::default_hook
   4: std::panicking::rust_panic_with_hook
   5: std::panicking::continue_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: mumble_web_proxy::main
  10: std::rt::lang_start::{{closure}}
  11: std::panicking::try::do_call
  12: __rust_maybe_catch_panic
  13: std::rt::lang_start_internal
  14: main
  15: __libc_start_main
  16: _start

I'm not sure if this is a mumble-web-proxy bug, a bug in some rust library or something else.

Johni0702 commented 5 years ago

This is an issue with mumble-web-proxy.

It's quality is currently only barely at PoC level. Properly handling invalid data should be fairly easy though. I just haven't gotten around to it because it wasn't necessary for the PoC (so atm they just crash the program rather than causing undefined behavior) and I'd rather replace the lib used for ICE first (because those are badly written and unmaintained C bindings and could potentially cause more serious issues than just panics).

I wouldn't recommend running this in production until both of above issues are fixed. I'll keep this issue open as a reminder until then.

Johni0702 commented 5 years ago

Turns out this particular panic was actually caused by the websocket crate and looking at its issue tracker, there are similar issues (some of which unresolved for over a year already), so I've decided to switch to tungstenite as the websocket server implementation (above commit).

I've also replaced the lib used for ICE with a new one and cleaned up the message parsing and related error handling.

While I wouldn't call this production ready quite yet (I ran it like ten times, and only with FF; more testing is definitely welcome), it should stop crashing constantly now and I am considering replacing my demo server's websockify and hosting a demo instance of mumble-web's webrtc branch (actually, I still need to implement NAT-related features before I can do that).