search

mozilla / janus-plugin-rs

Rust crate for wrapping the Janus C plugin API, so you can build Janus plugins in Rust.
Mozilla Public License 2.0
46 stars 18 forks source link

Unsound usages of unsafe implementation from `node.data` to `RawAttribute` #32

Closed llooFlashooll closed 3 weeks ago

llooFlashooll commented 3 weeks ago

Hi, I am scanning the janus-plugin in the latest version with my own static analyzer tool.

Unsafe conversion found at: src/sdp.rs#L222

   while let Some(node) = attr_node.as_ref() {
      let next = node.next; // we might delete this link, so grab next now!
      let data = node.data as *mut RawAttribute;
      let attr = data.as_ref().expect("Null data in SDP attribute node :(");
      let name = CStr::from_ptr(attr.name).to_str().expect("Invalid attribute name in SDP :(");

This unsound implementation would create a misalignment issues if the type size of node.data is smaller than the type size of RawAttribute.

This would potentially cause undefined behaviors in Rust. If we further manipulate the problematic converted types, it would potentially lead to different consequences such as access out-of-bound. I am reporting this issue for your attention.

vincentfretin commented 3 weeks ago

Hi, Please note this repo is not maintained anymore by mozilla, hubs switched to mediasoup stack a long time ago. The networked-aframe community forked it, so latest versions are in https://github.com/networked-aframe/janus-plugin-rs and https://github.com/networked-aframe/janus-plugin-sfu

The node.data is a janus_sdp_attribute struct, that struct is allocated on the C side by janus-gateway core when using ffi::sdp::janus_sdp_attribute_create. I don't see any real issue here.

llooFlashooll commented 3 weeks ago

Thanks for your kind and reasonable response! My tool may have some FP issues. That is my bad.