linklayer / cantact

Drivers and Command Line Interface for CANtact tools
http://cantact.io
MIT License
46 stars 12 forks source link

No input validation on ./can send #9

Closed brainstorm closed 2 months ago

brainstorm commented 3 years ago

Using a purchased Cantact Pro here...

cantact % ./target/debug/can send 0 CFE6CEB CACA
thread 'main' panicked at 'range end index 64 out of range for slice of length 0', driver/src/lib.rs:100:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
zsh: segmentation fault  ./target/debug/can send 0 CFE6CEB CACA

Bad input should return a Result<Err> with a helpful error message instead of panic-ing... also, I'd appreciate some example of send command (with output) in the docs :-S

brainstorm commented 3 years ago

More backtrace details:

% ./target/debug/can send -e 0 0 0
thread 'main' panicked at 'range end index 64 out of range for slice of length 0', driver/src/lib.rs:100:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5404efc28a0cddee103ef6396c48ea71ff9631c8/library/std/src/panicking.rs:495:5
   1: core::panicking::panic_fmt
             at /rustc/5404efc28a0cddee103ef6396c48ea71ff9631c8/library/core/src/panicking.rs:92:14
   2: core::slice::index::slice_end_index_len_fail
             at /rustc/5404efc28a0cddee103ef6396c48ea71ff9631c8/library/core/src/slice/index.rs:41:5
   3: <core::ops::range::Range<usize> as core::slice::index::SliceIndex<[T]>>::index
             at /Users/rvalls/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/slice/index.rs:238:13
   4: <core::ops::range::RangeTo<usize> as core::slice::index::SliceIndex<[T]>>::index
             at /Users/rvalls/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/slice/index.rs:284:9
   5: core::slice::index::<impl core::ops::index::Index<I> for [T]>::index
             at /Users/rvalls/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/slice/index.rs:15:9
   6: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /Users/rvalls/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/vec.rs:1939:9
   7: cantact::Frame::data_as_array
             at ./driver/src/lib.rs:100:38
   8: cantact::Frame::to_host_frame
             at ./driver/src/lib.rs:130:19
   9: cantact::Interface::send
             at ./driver/src/lib.rs:507:23
  10: can::send::cmd
             at ./src/send.rs:24:9
  11: can::main
             at ./src/main.rs:38:30
  12: core::ops::function::FnOnce::call_once
             at /Users/rvalls/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
zsh: segmentation fault  ./target/debug/can send -e 0 0 0
brainstorm commented 3 years ago

I just made a few quick changes:

diff --git a/driver/src/c/mod.rs b/driver/src/c/mod.rs
index 2a6f746..6ab87d9 100644
--- a/driver/src/c/mod.rs
+++ b/driver/src/c/mod.rs
@@ -33,7 +33,7 @@ impl CFrame {
             channel: f.channel,
             id: f.can_id,
             dlc: f.can_dlc,
-            data: f.data_as_array(),
+            data: f.data,
             ext: if f.ext { 1 } else { 0 },
             fd: if f.fd { 1 } else { 0 },
             brs: if f.brs { 1 } else { 0 },
@@ -153,7 +153,7 @@ pub unsafe extern "C" fn cantact_transmit(ptr: *mut CInterface, cf: CFrame) -> i
         channel: 0, //cf.channel,
         can_id: cf.id,
         can_dlc: cf.dlc,
-        data: cf.data.to_vec(),
+        data: cf.data,
         ext: cf.ext > 0,
         fd: cf.fd > 0,
         brs: cf.brs > 0,
diff --git a/driver/src/device/mod.rs b/driver/src/device/mod.rs
index 18d89c0..c878d80 100644
--- a/driver/src/device/mod.rs
+++ b/driver/src/device/mod.rs
@@ -402,7 +402,7 @@ impl Device {
     pub(crate) fn recv(&self) -> HostFrame {
         match self.can_rx_recv.recv() {
             Ok(f) => f,
-            Err(e) => panic!(e),
+            Err(e) => panic!("{}", e),
         }
     }
 }
diff --git a/driver/src/lib.rs b/driver/src/lib.rs
index 96ff41b..22a0b28 100644
--- a/driver/src/lib.rs
+++ b/driver/src/lib.rs
@@ -66,7 +66,7 @@ pub struct Frame {
     pub channel: u8,

     /// Frame data contents.
-    pub data: Vec<u8>,
+    pub data: [u8; 64],

     /// Extended (29 bit) arbitration identifier if true,
     /// standard (11 bit) arbitration identifer if false.
@@ -95,11 +95,6 @@ pub struct Frame {
     pub timestamp: Option<time::Duration>,
 }
 impl Frame {
-    fn data_as_array(&self) -> [u8; 64] {
-        let mut data = [0u8; 64];
-        data[..64].clone_from_slice(&self.data[..64]);
-        data
-    }
     // convert to a frame format expected by the device
     fn to_host_frame(&self) -> HostFrame {
         // if frame is extended, set the extended bit in host frame CAN ID
@@ -127,7 +122,7 @@ impl Frame {
             can_id,
             can_dlc: self.can_dlc,
             channel: self.channel,
-            data: self.data_as_array(),
+            data: self.data,
         }
     }
     /// Returns a default CAN frame with all values set to zero/false.
@@ -135,7 +130,7 @@ impl Frame {
         Frame {
             can_id: 0,
             can_dlc: 0,
-            data: vec![],
+            data: [0; 64],
             channel: 0,
             ext: false,
             fd: false,
@@ -166,7 +161,7 @@ impl Frame {
         Frame {
             can_id,
             can_dlc: hf.can_dlc,
-            data: hf.data.to_vec(),
+            data: hf.data,
             channel: hf.channel,
             ext,
             loopback,
@@ -504,7 +499,7 @@ impl Interface {
             return Err(Error::NotRunning);
         }

-        self.dev.send(f.to_host_frame()).unwrap();
+        self.dev.send(f.to_host_frame())?;
         Ok(())
     }

diff --git a/src/send.rs b/src/send.rs
index 24532a8..a87a44b 100644
--- a/src/send.rs
+++ b/src/send.rs
@@ -21,7 +21,7 @@ pub fn cmd(_matches: &ArgMatches) -> Result<(), Error> {
     f.can_dlc = 8;
     loop {
         f.can_id = count % 0x800;
-        i.send(f.clone()).unwrap();
+        i.send(f.clone())?;
         count += 1;
         if count % 1000 == 0 {
             println!("{}", count)

And it does not panic, sends some incrementing IDs, but of course the payload is all 0's because the CLI needs more work, but hopefully you see what I mean @ericevenchick ?:

% cargo run send 0 0xCAFE 0xCAFE
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `target/debug/can send 0 0xCAFE 0xCAFE`
1000
2000
3000
4000
5000
6000
(...)

IMG_1216

Will you keep supporting this software after your hardware has shipped via CrowdSupply?