Do not attempt to validate length of packets being emitted.

This is a form of an uninitialized read bug; although safe it caused
panics. In short, transmit buffers received from the network stack
should be considered uninitialized (in practice they will often
contain previously transmitted packets or parts thereof). Wrapping
them with the only method we had (e.g. Ipv4Packet) treated the buffer
as if it contained a valid incoming packet, which can easily fail
with Error::Truncated.

This commit splits every `fn new(buffer: T) -> Result<Self, Error>`
method on a `Packet` into three smaller ones:
  * `fn check_len(&self) -> Result<(), Error>`, purely a validator;
  * `fn new(T) -> Self`, purely a wrapper;
  * `fn new_checked(T) -> Result<Self, Error>`, a validating wrapper.

This makes it easy to process ingress packets (using `new_checked`),
egress packets (using `new`), and, if needed, maintain the invariants
at any point during packet construction (using `check_len`).

Fixes #17.

examples/ping.rs

@@ -84,9 +84,9 @@ fn main() {
                     .send(ipv4_repr.buffer_len() + icmp_repr.buffer_len())
                     .unwrap();
 
-                let mut ipv4_packet = Ipv4Packet::new(raw_payload).unwrap();
+                let mut ipv4_packet = Ipv4Packet::new(raw_payload);
                 ipv4_repr.emit(&mut ipv4_packet);
-                let mut icmp_packet = Icmpv4Packet::new(ipv4_packet.payload_mut()).unwrap();
+                let mut icmp_packet = Icmpv4Packet::new(ipv4_packet.payload_mut());
                 icmp_repr.emit(&mut icmp_packet);
 
                 waiting_queue.insert(seq_no, timestamp);
@@ -96,11 +96,11 @@ fn main() {
 
             if socket.can_recv() {
                 let payload = socket.recv().unwrap();
-                let ipv4_packet = Ipv4Packet::new(payload).unwrap();
+                let ipv4_packet = Ipv4Packet::new(payload);
                 let ipv4_repr = Ipv4Repr::parse(&ipv4_packet).unwrap();
 
                 if ipv4_repr.src_addr == remote_addr && ipv4_repr.dst_addr == local_addr {
-                    let icmp_packet = Icmpv4Packet::new(ipv4_packet.payload()).unwrap();
+                    let icmp_packet = Icmpv4Packet::new(ipv4_packet.payload());
                     let icmp_repr = Icmpv4Repr::parse(&icmp_packet);
 
                     if let Ok(Icmpv4Repr::EchoReply { seq_no, data, .. }) = icmp_repr {

src/iface/ethernet.rs

@@ -117,7 +117,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
 
         // Now, receive any incoming packets.
         let rx_buffer = try!(self.device.receive());
-        let eth_frame = try!(EthernetFrame::new(&rx_buffer));
+        let eth_frame = try!(EthernetFrame::new_checked(&rx_buffer));
 
         if !eth_frame.dst_addr().is_broadcast() &&
                 eth_frame.dst_addr() != self.hardware_addr {
@@ -128,7 +128,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
         match eth_frame.ethertype() {
             // Snoop all ARP traffic, and respond to ARP packets directed at us.
             EthernetProtocol::Arp => {
-                let arp_packet = try!(ArpPacket::new(eth_frame.payload()));
+                let arp_packet = try!(ArpPacket::new_checked(eth_frame.payload()));
                 match try!(ArpRepr::parse(&arp_packet)) {
                     // Respond to ARP requests aimed at us, and fill the ARP cache
                     // from all ARP requests, including gratuitous.
@@ -170,7 +170,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
 
             // Handle IP packets directed at us.
             EthernetProtocol::Ipv4 => {
-                let ipv4_packet = try!(Ipv4Packet::new(eth_frame.payload()));
+                let ipv4_packet = try!(Ipv4Packet::new_checked(eth_frame.payload()));
                 let ipv4_repr = try!(Ipv4Repr::parse(&ipv4_packet));
 
                 // Fill the ARP cache from IP header.
@@ -195,7 +195,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
 
                     // Respond to ICMP packets.
                     Ipv4Repr { protocol: IpProtocol::Icmp, src_addr, dst_addr, .. } => {
-                        let icmp_packet = try!(Icmpv4Packet::new(ipv4_packet.payload()));
+                        let icmp_packet = try!(Icmpv4Packet::new_checked(ipv4_packet.payload()));
                         let icmp_repr = try!(Icmpv4Repr::parse(&icmp_packet));
                         match icmp_repr {
                             // Respond to echo requests.
@@ -251,7 +251,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                         }
 
                         if !handled && protocol == IpProtocol::Tcp {
-                            let tcp_packet = try!(TcpPacket::new(ipv4_packet.payload()));
+                            let tcp_packet = try!(TcpPacket::new_checked(ipv4_packet.payload()));
                             if !tcp_packet.rst() {
                                 let tcp_reply_repr = TcpRepr {
                                     src_port:     tcp_packet.dst_port(),
@@ -312,12 +312,13 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                 let frame_len = EthernetFrame::<&[u8]>::buffer_len($ip_repr.buffer_len() +
                                                                    $ip_repr.payload_len);
                 $tx_buffer = try!(self.device.transmit(frame_len));
-                $frame = try!(EthernetFrame::new(&mut $tx_buffer));
+                $frame = EthernetFrame::new_checked(&mut $tx_buffer)
+                                       .expect("transmit frame too small");
                 $frame.set_src_addr(self.hardware_addr);
                 $frame.set_dst_addr(dst_hardware_addr);
                 $frame.set_ethertype(EthernetProtocol::Ipv4);
 
-                let mut ip_packet = try!(Ipv4Packet::new($frame.payload_mut()));
+                let mut ip_packet = Ipv4Packet::new($frame.payload_mut());
                 $ip_repr.emit(&mut ip_packet);
                 ip_packet
             })
@@ -327,7 +328,8 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
             Response::Arp(repr) => {
                 let tx_len = EthernetFrame::<&[u8]>::buffer_len(repr.buffer_len());
                 let mut tx_buffer = try!(self.device.transmit(tx_len));
-                let mut frame = try!(EthernetFrame::new(&mut tx_buffer));
+                let mut frame = EthernetFrame::new_checked(&mut tx_buffer)
+                                              .expect("transmit frame too small");
                 frame.set_src_addr(self.hardware_addr);
                 frame.set_dst_addr(match repr {
                     ArpRepr::EthernetIpv4 { target_hardware_addr, .. } => target_hardware_addr,
@@ -335,7 +337,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                 });
                 frame.set_ethertype(EthernetProtocol::Arp);
 
-                let mut packet = try!(ArpPacket::new(frame.payload_mut()));
+                let mut packet = ArpPacket::new(frame.payload_mut());
                 repr.emit(&mut packet);
 
                 Ok(())
@@ -345,7 +347,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                 let mut tx_buffer;
                 let mut frame;
                 let mut ip_packet = ip_response!(tx_buffer, frame, ip_repr);
-                let mut icmp_packet = try!(Icmpv4Packet::new(ip_packet.payload_mut()));
+                let mut icmp_packet = Icmpv4Packet::new(ip_packet.payload_mut());
                 icmp_repr.emit(&mut icmp_packet);
                 Ok(())
             }
@@ -354,7 +356,7 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                 let mut tx_buffer;
                 let mut frame;
                 let mut ip_packet = ip_response!(tx_buffer, frame, ip_repr);
-                let mut tcp_packet = try!(TcpPacket::new(ip_packet.payload_mut()));
+                let mut tcp_packet = TcpPacket::new(ip_packet.payload_mut());
                 tcp_repr.emit(&mut tcp_packet,
                               &IpAddress::Ipv4(ip_repr.src_addr),
                               &IpAddress::Ipv4(ip_repr.dst_addr));
@@ -388,14 +390,15 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
                         let tx_len = EthernetFrame::<&[u8]>::buffer_len(repr.buffer_len() +
                                                                         payload.buffer_len());
                         let mut tx_buffer = try!(device.transmit(tx_len));
-                        let mut frame = try!(EthernetFrame::new(&mut tx_buffer));
+                        let mut frame = EthernetFrame::new_checked(&mut tx_buffer)
+                                                      .expect("transmit frame too small");
                         frame.set_src_addr(src_hardware_addr);
                         frame.set_dst_addr(dst_hardware_addr);
                         frame.set_ethertype(EthernetProtocol::Ipv4);
 
                         repr.emit(frame.payload_mut());
 
-                        let mut ip_packet = try!(Ipv4Packet::new(frame.payload_mut()));
+                        let mut ip_packet = Ipv4Packet::new(frame.payload_mut());
                         payload.emit(&repr, ip_packet.payload_mut());
                     }
 
@@ -418,12 +421,13 @@ impl<'a, 'b, 'c, DeviceT: Device + 'a> Interface<'a, 'b, 'c, DeviceT> {
 
                         let tx_len = EthernetFrame::<&[u8]>::buffer_len(payload.buffer_len());
                         let mut tx_buffer = try!(device.transmit(tx_len));
-                        let mut frame = try!(EthernetFrame::new(&mut tx_buffer));
+                        let mut frame = EthernetFrame::new_checked(&mut tx_buffer)
+                                                      .expect("transmit frame too small");
                         frame.set_src_addr(src_hardware_addr);
                         frame.set_dst_addr(EthernetAddress([0xff; 6]));
                         frame.set_ethertype(EthernetProtocol::Arp);
 
-                        let mut arp_packet = try!(ArpPacket::new(frame.payload_mut()));
+                        let mut arp_packet = ArpPacket::new(frame.payload_mut());
                         payload.emit(&mut arp_packet);
                     }
                 }

src/lib.rs

@@ -112,8 +112,7 @@ mod parsing;
 /// The error type for the networking stack.
 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 pub enum Error {
-    /// An incoming packet could not be parsed, or an outgoing packet could not be emitted
-    /// because a field was out of bounds for the underlying buffer.
+    /// An incoming packet could not be parsed.
     Truncated,
     /// An incoming packet could not be recognized and was dropped.
     /// E.g. a packet with an unknown EtherType.

src/socket/raw.rs

@@ -179,10 +179,10 @@ impl<'a, 'b> RawSocket<'a, 'b> {
 
         match self.ip_version {
             IpVersion::Ipv4 => {
-                let mut ipv4_packet = Ipv4Packet::new(packet_buf.as_mut())?;
+                let mut ipv4_packet = Ipv4Packet::new_checked(packet_buf.as_mut())?;
                 ipv4_packet.fill_checksum();
 
-                let ipv4_packet = Ipv4Packet::new(&*ipv4_packet.into_inner())?;
+                let ipv4_packet = Ipv4Packet::new(&*ipv4_packet.into_inner());
                 let raw_repr = RawRepr(ipv4_packet.payload());
                 let ipv4_repr = Ipv4Repr::parse(&ipv4_packet)?;
                 emit(&IpRepr::Ipv4(ipv4_repr), &raw_repr)

src/socket/tcp.rs

@@ -636,7 +636,7 @@ impl<'a> TcpSocket<'a> {
 
         if ip_repr.protocol() != IpProtocol::Tcp { return Err(Error::Rejected) }
 
-        let packet = try!(TcpPacket::new(&payload[..ip_repr.payload_len()]));
+        let packet = try!(TcpPacket::new_checked(&payload[..ip_repr.payload_len()]));
         let repr = try!(TcpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr()));
 
         // Reject packets with a wrong destination.
@@ -1131,7 +1131,7 @@ impl<'a> IpPayload for TcpRepr<'a> {
     }
 
     fn emit(&self, ip_repr: &IpRepr, payload: &mut [u8]) {
-        let mut packet = TcpPacket::new(payload).expect("undersized payload");
+        let mut packet = TcpPacket::new(payload);
         self.emit(&mut packet, &ip_repr.src_addr(), &ip_repr.dst_addr())
     }
 }
@@ -1216,7 +1216,7 @@ mod test {
     fn send(socket: &mut TcpSocket, timestamp: u64, repr: &TcpRepr) -> Result<(), Error> {
         trace!("send: {}", repr);
         let mut buffer = vec![0; repr.buffer_len()];
-        let mut packet = TcpPacket::new(&mut buffer).unwrap();
+        let mut packet = TcpPacket::new(&mut buffer);
         repr.emit(&mut packet, &REMOTE_IP, &LOCAL_IP);
         let ip_repr = IpRepr::Unspecified {
             src_addr:    REMOTE_IP,
@@ -1239,7 +1239,7 @@ mod test {
 
             buffer.resize(payload.buffer_len(), 0);
             payload.emit(&ip_repr, &mut buffer[..]);
-            let packet = TcpPacket::new(&buffer[..]).unwrap();
+            let packet = TcpPacket::new(&buffer[..]);
             let repr = try!(TcpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr()));
             trace!("recv: {}", repr);
             Ok(f(Ok(repr)))
@@ -2565,7 +2565,7 @@ mod test {
         s.dispatch(0, &limits, &mut |ip_repr, payload| {
             let mut buffer = vec![0; payload.buffer_len()];
             payload.emit(&ip_repr, &mut buffer[..]);
-            let packet = TcpPacket::new(&buffer[..]).unwrap();
+            let packet = TcpPacket::new(&buffer[..]);
             assert_eq!(packet.window_len(), 32767);
             Ok(())
         }).unwrap();
@@ -2575,7 +2575,7 @@ mod test {
         s.dispatch(0, &limits, &mut |ip_repr, payload| {
             let mut buffer = vec![0; payload.buffer_len()];
             payload.emit(&ip_repr, &mut buffer[..]);
-            let packet = TcpPacket::new(&buffer[..]).unwrap();
+            let packet = TcpPacket::new(&buffer[..]);
             assert_eq!(packet.window_len(), 5920);
             Ok(())
         }).unwrap();

src/socket/udp.rs

@@ -155,7 +155,7 @@ impl<'a, 'b> UdpSocket<'a, 'b> {
                    payload: &[u8]) -> Result<(), Error> {
         if ip_repr.protocol() != IpProtocol::Udp { return Err(Error::Rejected) }
 
-        let packet = try!(UdpPacket::new(&payload[..ip_repr.payload_len()]));
+        let packet = try!(UdpPacket::new_checked(&payload[..ip_repr.payload_len()]));
         let repr = try!(UdpRepr::parse(&packet, &ip_repr.src_addr(), &ip_repr.dst_addr()));
 
         if repr.dst_port != self.endpoint.port { return Err(Error::Rejected) }
@@ -202,7 +202,7 @@ impl<'a> IpPayload for UdpRepr<'a> {
     }
 
     fn emit(&self, repr: &IpRepr, payload: &mut [u8]) {
-        let mut packet = UdpPacket::new(payload).expect("undersized payload");
+        let mut packet = UdpPacket::new(payload);
         self.emit(&mut packet, &repr.src_addr(), &repr.dst_addr())
     }
 }

src/wire/arp.rs

@@ -62,23 +62,41 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Packet<T> {
-    /// Wrap a buffer with an ARP packet. Returns an error if the buffer
-    /// is too small to contain one.
-    pub fn new(buffer: T) -> Result<Packet<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with ARP packet structure.
+    pub fn new(buffer: T) -> Packet<T> {
+        Packet { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Packet<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    ///
+    /// The result of this check is invalidated by calling [set_hardware_len] or
+    /// [set_protocol_len].
+    ///
+    /// [set_hardware_len]: #method.set_hardware_len
+    /// [set_protocol_len]: #method.set_protocol_len
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::OPER.end {
             Err(Error::Truncated)
+        } else if len < field::TPA(self.hardware_len(), self.protocol_len()).end {
+            Err(Error::Truncated)
         } else {
-            let packet = Packet { buffer: buffer };
-            if len < field::TPA(packet.hardware_len(), packet.protocol_len()).end {
-                Err(Error::Truncated)
-            } else {
-                Ok(packet)
-            }
+            Ok(())
         }
     }
 
-    /// Consumes the packet, returning the underlying buffer.
+    /// Consume the packet, returning the underlying buffer.
     pub fn into_inner(self) -> T {
         self.buffer
     }
@@ -336,7 +354,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        match Packet::new(buffer) {
+        match Packet::new_checked(buffer) {
             Err(err)  => write!(f, "{}({})\n", indent, err),
             Ok(frame) => write!(f, "{}{}\n", indent, frame)
         }
@@ -360,7 +378,7 @@ mod test {
 
     #[test]
     fn test_deconstruct() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         assert_eq!(packet.hardware_type(), Hardware::Ethernet);
         assert_eq!(packet.protocol_type(), Protocol::Ipv4);
         assert_eq!(packet.hardware_len(), 6);
@@ -375,7 +393,7 @@ mod test {
     #[test]
     fn test_construct() {
         let mut bytes = vec![0; 28];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet.set_hardware_type(Hardware::Ethernet);
         packet.set_protocol_type(Protocol::Ipv4);
         packet.set_hardware_len(6);
@@ -404,7 +422,7 @@ mod test {
 
     #[test]
     fn test_parse() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         let repr = Repr::parse(&packet).unwrap();
         assert_eq!(repr, packet_repr());
     }
@@ -412,7 +430,7 @@ mod test {
     #[test]
     fn test_emit() {
         let mut bytes = vec![0; 28];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet_repr().emit(&mut packet);
         assert_eq!(&packet.into_inner()[..], &PACKET_BYTES[..]);
     }

src/wire/ethernet.rs

@@ -91,14 +91,29 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Frame<T> {
-    /// Wrap a buffer with an Ethernet frame. Returns an error if the buffer
-    /// is too small or too large to contain one.
-    pub fn new(buffer: T) -> Result<Frame<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with Ethernet frame structure.
+    pub fn new(buffer: T) -> Frame<T> {
+        Frame { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Frame<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::PAYLOAD.start {
             Err(Error::Truncated)
         } else {
-            Ok(Frame { buffer: buffer })
+            Ok(())
         }
     }
 
@@ -194,7 +209,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Frame<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        let frame = match Frame::new(buffer) {
+        let frame = match Frame::new_checked(buffer) {
             Err(err)  => return write!(f, "{}({})\n", indent, err),
             Ok(frame) => frame
         };
@@ -238,7 +253,7 @@ mod test {
 
     #[test]
     fn test_deconstruct() {
-        let frame = Frame::new(&FRAME_BYTES[..]).unwrap();
+        let frame = Frame::new(&FRAME_BYTES[..]);
         assert_eq!(frame.dst_addr(), Address([0x01, 0x02, 0x03, 0x04, 0x05, 0x06]));
         assert_eq!(frame.src_addr(), Address([0x11, 0x12, 0x13, 0x14, 0x15, 0x16]));
         assert_eq!(frame.ethertype(), EtherType::Ipv4);
@@ -248,7 +263,7 @@ mod test {
     #[test]
     fn test_construct() {
         let mut bytes = vec![0; 64];
-        let mut frame = Frame::new(&mut bytes).unwrap();
+        let mut frame = Frame::new(&mut bytes);
         frame.set_dst_addr(Address([0x01, 0x02, 0x03, 0x04, 0x05, 0x06]));
         frame.set_src_addr(Address([0x11, 0x12, 0x13, 0x14, 0x15, 0x16]));
         frame.set_ethertype(EtherType::Ipv4);

src/wire/icmpv4.rs

@@ -184,23 +184,41 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Packet<T> {
-    /// Wrap a buffer with an ICMPv4 packet. Returns an error if the buffer
-    /// is too small to contain one.
-    pub fn new(buffer: T) -> Result<Packet<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with ICMPv4 packet structure.
+    pub fn new(buffer: T) -> Packet<T> {
+        Packet { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Packet<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    ///
+    /// The result of this check is invalidated by calling [set_header_len].
+    ///
+    /// [set_header_len]: #method.set_header_len
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::CHECKSUM.end {
             Err(Error::Truncated)
         } else {
-            let packet = Packet { buffer: buffer };
-            if len < packet.header_len() {
+            if len < self.header_len() as usize {
                 Err(Error::Truncated)
             } else {
-                Ok(packet)
+                Ok(())
             }
         }
     }
 
-    /// Consumes the packet, returning the underlying buffer.
+    /// Consume the packet, returning the underlying buffer.
     pub fn into_inner(self) -> T {
         self.buffer
     }
@@ -380,7 +398,7 @@ impl<'a> Repr<'a> {
             },
 
             (Message::DstUnreachable, code) => {
-                let ip_packet = try!(Ipv4Packet::new(packet.data()));
+                let ip_packet = try!(Ipv4Packet::new_checked(packet.data()));
 
                 let payload = &packet.data()[ip_packet.header_len() as usize..];
                 // RFC 792 requires exactly eight bytes to be returned.
@@ -443,8 +461,7 @@ impl<'a> Repr<'a> {
                 packet.set_msg_type(Message::DstUnreachable);
                 packet.set_msg_code(reason.into());
 
-                let mut ip_packet = Ipv4Packet::new(packet.data_mut())
-                                               .expect("undersized data");
+                let mut ip_packet = Ipv4Packet::new(packet.data_mut());
                 header.emit(&mut ip_packet);
                 let mut payload = &mut ip_packet.into_inner()[header.buffer_len()..];
                 payload.copy_from_slice(&data[..])
@@ -496,7 +513,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        let packet = match Packet::new(buffer) {
+        let packet = match Packet::new_checked(buffer) {
             Err(err)   => return write!(f, "{}({})\n", indent, err),
             Ok(packet) => packet
         };
@@ -525,7 +542,7 @@ mod test {
 
     #[test]
     fn test_echo_deconstruct() {
-        let packet = Packet::new(&ECHO_PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&ECHO_PACKET_BYTES[..]);
         assert_eq!(packet.msg_type(), Message::EchoRequest);
         assert_eq!(packet.msg_code(), 0);
         assert_eq!(packet.checksum(), 0x8efe);
@@ -538,7 +555,7 @@ mod test {
     #[test]
     fn test_echo_construct() {
         let mut bytes = vec![0; 12];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet.set_msg_type(Message::EchoRequest);
         packet.set_msg_code(0);
         packet.set_echo_ident(0x1234);
@@ -558,7 +575,7 @@ mod test {
 
     #[test]
     fn test_echo_parse() {
-        let packet = Packet::new(&ECHO_PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&ECHO_PACKET_BYTES[..]);
         let repr = Repr::parse(&packet).unwrap();
         assert_eq!(repr, echo_packet_repr());
     }
@@ -567,7 +584,7 @@ mod test {
     fn test_echo_emit() {
         let repr = echo_packet_repr();
         let mut bytes = vec![0; repr.buffer_len()];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         repr.emit(&mut packet);
         assert_eq!(&packet.into_inner()[..], &ECHO_PACKET_BYTES[..]);
     }

src/wire/ip.rs

@@ -267,9 +267,12 @@ impl IpRepr {
     /// This function panics if invoked on an unspecified representation.
     pub fn buffer_len(&self) -> usize {
         match self {
-            &IpRepr::Unspecified { .. } => panic!("unspecified IP representation"),
-            &IpRepr::Ipv4(repr) => repr.buffer_len(),
-            &IpRepr::__Nonexhaustive => unreachable!()
+            &IpRepr::Unspecified { .. } =>
+                panic!("unspecified IP representation"),
+            &IpRepr::Ipv4(repr) =>
+                repr.buffer_len(),
+            &IpRepr::__Nonexhaustive =>
+                unreachable!()
         }
     }
 
@@ -279,12 +282,12 @@ impl IpRepr {
     /// This function panics if invoked on an unspecified representation.
     pub fn emit<T: AsRef<[u8]> + AsMut<[u8]>>(&self, buffer: T) {
         match self {
-            &IpRepr::Unspecified { .. } => panic!("unspecified IP representation"),
-            &IpRepr::Ipv4(repr) => {
-                let mut packet = Ipv4Packet::new(buffer).expect("undersized buffer");
-                repr.emit(&mut packet)
-            }
-            &IpRepr::__Nonexhaustive => unreachable!()
+            &IpRepr::Unspecified { .. } =>
+                panic!("unspecified IP representation"),
+            &IpRepr::Ipv4(repr) =>
+                repr.emit(&mut Ipv4Packet::new(buffer)),
+            &IpRepr::__Nonexhaustive =>
+                unreachable!()
         }
     }
 }

src/wire/ipv4.rs

@@ -97,23 +97,41 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Packet<T> {
-    /// Wrap a buffer with an IPv4 packet. Returns an error if the buffer
-    /// is too small to contain one.
-    pub fn new(buffer: T) -> Result<Packet<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with IPv4 packet structure.
+    pub fn new(buffer: T) -> Packet<T> {
+        Packet { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Packet<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    ///
+    /// The result of this check is invalidated by calling [set_header_len].
+    ///
+    /// [set_header_len]: #method.set_header_len
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::DST_ADDR.end {
             Err(Error::Truncated)
         } else {
-            let packet = Packet { buffer: buffer };
-            if len < packet.header_len() as usize {
+            if len < self.header_len() as usize {
                 Err(Error::Truncated)
             } else {
-                Ok(packet)
+                Ok(())
             }
         }
     }
 
-    /// Consumes the packet, returning the underlying buffer.
+    /// Consume the packet, returning the underlying buffer.
     pub fn into_inner(self) -> T {
         self.buffer
     }
@@ -477,7 +495,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        let (ip_repr, payload) = match Packet::new(buffer) {
+        let (ip_repr, payload) = match Packet::new_checked(buffer) {
             Err(err) => return write!(f, "{}({})\n", indent, err),
             Ok(ip_packet) => {
                 try!(write!(f, "{}{}\n", indent, ip_packet));
@@ -493,7 +511,7 @@ impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
             Protocol::Icmp =>
                 super::Icmpv4Packet::<&[u8]>::pretty_print(&payload, f, indent),
             Protocol::Udp => {
-                match super::UdpPacket::new(payload) {
+                match super::UdpPacket::new_checked(payload) {
                     Err(err) => write!(f, "{}({})\n", indent, err),
                     Ok(udp_packet) => {
                         match super::UdpRepr::parse(&udp_packet,
@@ -506,7 +524,7 @@ impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
                 }
             }
             Protocol::Tcp => {
-                match super::TcpPacket::new(payload) {
+                match super::TcpPacket::new_checked(payload) {
                     Err(err) => write!(f, "{}({})\n", indent, err),
                     Ok(tcp_packet) => {
                         match super::TcpRepr::parse(&tcp_packet,
@@ -544,7 +562,7 @@ mod test {
 
     #[test]
     fn test_deconstruct() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         assert_eq!(packet.version(), 4);
         assert_eq!(packet.header_len(), 20);
         assert_eq!(packet.dscp(), 0);
@@ -566,7 +584,7 @@ mod test {
     #[test]
     fn test_construct() {
         let mut bytes = vec![0; 30];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet.set_version(4);
         packet.set_header_len(20);
         packet.set_dscp(0);
@@ -607,7 +625,7 @@ mod test {
 
     #[test]
     fn test_parse() {
-        let packet = Packet::new(&REPR_PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&REPR_PACKET_BYTES[..]);
         let repr = Repr::parse(&packet).unwrap();
         assert_eq!(repr, packet_repr());
     }
@@ -616,7 +634,7 @@ mod test {
     fn test_emit() {
         let repr = packet_repr();
         let mut bytes = vec![0; repr.buffer_len() + REPR_PAYLOAD_BYTES.len()];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         repr.emit(&mut packet);
         packet.payload_mut().copy_from_slice(&REPR_PAYLOAD_BYTES);
         assert_eq!(&packet.into_inner()[..], &REPR_PACKET_BYTES[..]);

src/wire/mod.rs

@@ -4,22 +4,41 @@
 //! of functionality.
 //!
 //!  * First, it provides functions to extract fields from sequences of octets,
-//!    and to insert fields into sequences of octets. This happens through the `Frame`
-//!    and `Packet` families of structures, e.g. [EthernetPacket](struct.EthernetPacket.html).
+//!    and to insert fields into sequences of octets. This happens `Packet` family of
+//!    structures, e.g. [EthernetFrame] or [Ipv4Packet].
 //!  * Second, in cases where the space of valid field values is much smaller than the space
 //!    of possible field values, it provides a compact, high-level representation
 //!    of packet data that can be parsed from and emitted into a sequence of octets.
-//!    This happens through the `Repr` family of enums, e.g. [ArpRepr](enum.ArpRepr.html).
+//!    This happens through the `Repr` family of structs and enums, e.g. [ArpRepr] or [Ipv4Repr].
 // https://github.com/rust-lang/rust/issues/38739
 //! </ul>
 //!
-//! The functions in the `wire` module are designed for robustness and use together with
-//! `-Cpanic=abort`. The accessor and parsing functions never panic. The setter and emission
-//! functions only panic if the underlying buffer is too small.
+//! [EthernetFrame]: struct.EthernetFrame.html
+//! [Ipv4Packet]: struct.Ipv4Packet.html
+//! [ArpRepr]: enum.ArpRepr.html
+//! [Ipv4Repr]: struct.Ipv4Repr.html
 //!
-//! The `Frame` and `Packet` families of data structures in the `wire` module do not perform
-//! validation of received data except as needed to access the contents without panicking;
-//! the `Repr` family does.
+//! The functions in the `wire` module are designed for use together with `-Cpanic=abort`.
+//!
+//! The `Packet` family of data structures guarantees that, if the `Packet::check_len()` method
+//! returned `Ok(())`, then no accessor or setter method will panic; however, the guarantee
+//! provided by `Packet::check_len()` may no longer hold after changing certain fields,
+//! which are listed in the documentation for the specific packet.
+//!
+//! The `Packet::new_checked` method is a shorthand for a combination of `Packet::new` and
+//! `Packet::check_len`.
+//! When parsing untrusted input, it is *necessary* to use `Packet::new_checked()`;
+//! so long as the buffer is not modified, no accessor will fail.
+//! When emitting output, though, it is *incorrect* to use `Packet::new_checked()`;
+//! the length check is likely to succeed on a zeroed buffer, but fail on a buffer
+//! filled with data from a previous packet, such as when reusing buffers, resulting
+//! in nondeterministic panics with some network devices but not others.
+//! The buffer length for emission is not calculated by the `Packet` layer.
+//!
+//! In the `Repr` family of data structures, the `Repr::parse()` method never panics
+//! as long as `Packet::new_checked()` (or `Packet::check_len()`) has succeeded, and
+//! the `Repr::emit()` method never panics as long as the underlying buffer is exactly
+//! `Repr::buffer_len()` octets long.
 //!
 //! # Examples
 //!
@@ -34,14 +53,16 @@ let repr = Ipv4Repr {
     protocol:    IpProtocol::Tcp,
     payload_len: 10
 };
-let mut buffer = vec![0; repr.buffer_len() + 10];
+let mut buffer = vec![0; repr.buffer_len() + repr.payload_len];
 { // emission
-    let mut packet = Ipv4Packet::new(&mut buffer).unwrap();
+    let mut packet = Ipv4Packet::new(&mut buffer);
     repr.emit(&mut packet);
 }
 { // parsing
-    let packet = Ipv4Packet::new(&buffer).unwrap();
-    let parsed = Ipv4Repr::parse(&packet).unwrap();
+    let packet = Ipv4Packet::new_checked(&buffer)
+                            .expect("truncated packet");
+    let parsed = Ipv4Repr::parse(&packet)
+                          .expect("malformed packet");
     assert_eq!(repr, parsed);
 }
 ```

src/wire/tcp.rs

@@ -97,18 +97,37 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Packet<T> {
-    /// Wrap a buffer with a TCP packet. Returns an error if the buffer
-    /// is too small to contain one.
-    pub fn new(buffer: T) -> Result<Packet<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with TCP packet structure.
+    pub fn new(buffer: T) -> Packet<T> {
+        Packet { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Packet<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    ///
+    /// The result of this check is invalidated by calling [set_header_len].
+    ///
+    /// [set_header_len]: #method.set_header_len
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::URGENT.end {
             Err(Error::Truncated)
         } else {
-            Ok(Packet { buffer: buffer })
+            Ok(())
         }
     }
 
-    /// Consumes the packet, returning the underlying buffer.
+    /// Consume the packet, returning the underlying buffer.
     pub fn into_inner(self) -> T {
         self.buffer
     }
@@ -761,7 +780,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        match Packet::new(buffer) {
+        match Packet::new_checked(buffer) {
             Err(err)   => write!(f, "{}({})\n", indent, err),
             Ok(packet) => write!(f, "{}{}\n", indent, packet)
         }
@@ -793,7 +812,7 @@ mod test {
 
     #[test]
     fn test_deconstruct() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         assert_eq!(packet.src_port(), 48896);
         assert_eq!(packet.dst_port(), 80);
         assert_eq!(packet.seq_number(), SeqNumber(0x01234567));
@@ -816,7 +835,7 @@ mod test {
     #[test]
     fn test_construct() {
         let mut bytes = vec![0; PACKET_BYTES.len()];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet.set_src_port(48896);
         packet.set_dst_port(80);
         packet.set_seq_number(SeqNumber(0x01234567));
@@ -860,7 +879,7 @@ mod test {
 
     #[test]
     fn test_parse() {
-        let packet = Packet::new(&SYN_PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&SYN_PACKET_BYTES[..]);
         let repr = Repr::parse(&packet, &SRC_ADDR.into(), &DST_ADDR.into()).unwrap();
         assert_eq!(repr, packet_repr());
     }
@@ -869,7 +888,7 @@ mod test {
     fn test_emit() {
         let repr = packet_repr();
         let mut bytes = vec![0; repr.buffer_len()];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         repr.emit(&mut packet, &SRC_ADDR.into(), &DST_ADDR.into());
         assert_eq!(&packet.into_inner()[..], &SYN_PACKET_BYTES[..]);
     }

src/wire/udp.rs

@@ -27,23 +27,41 @@ mod field {
 }
 
 impl<T: AsRef<[u8]>> Packet<T> {
-    /// Wrap a buffer with an UDP packet. Returns an error if the buffer
-    /// is too small to contain one.
-    pub fn new(buffer: T) -> Result<Packet<T>, Error> {
-        let len = buffer.as_ref().len();
+    /// Imbue a raw octet buffer with UDP packet structure.
+    pub fn new(buffer: T) -> Packet<T> {
+        Packet { buffer }
+    }
+
+    /// Shorthand for a combination of [new] and [check_len].
+    ///
+    /// [new]: #method.new
+    /// [check_len]: #method.check_len
+    pub fn new_checked(buffer: T) -> Result<Packet<T>, Error> {
+        let packet = Self::new(buffer);
+        try!(packet.check_len());
+        Ok(packet)
+    }
+
+    /// Ensure that no accessor method will panic if called.
+    /// Returns `Err(Error::Truncated)` if the buffer is too short.
+    ///
+    /// The result of this check is invalidated by calling [set_header_len].
+    ///
+    /// [set_header_len]: #method.set_header_len
+    pub fn check_len(&self) -> Result<(), Error> {
+        let len = self.buffer.as_ref().len();
         if len < field::CHECKSUM.end {
             Err(Error::Truncated)
         } else {
-            let packet = Packet { buffer: buffer };
-            if len < packet.len() as usize {
+            if len < self.len() as usize {
                 Err(Error::Truncated)
             } else {
-                Ok(packet)
+                Ok(())
             }
         }
     }
 
-    /// Consumes the packet, returning the underlying buffer.
+    /// Consume the packet, returning the underlying buffer.
     pub fn into_inner(self) -> T {
         self.buffer
     }
@@ -234,7 +252,7 @@ use super::pretty_print::{PrettyPrint, PrettyIndent};
 impl<T: AsRef<[u8]>> PrettyPrint for Packet<T> {
     fn pretty_print(buffer: &AsRef<[u8]>, f: &mut fmt::Formatter,
                     indent: &mut PrettyIndent) -> fmt::Result {
-        match Packet::new(buffer) {
+        match Packet::new_checked(buffer) {
             Err(err)   => write!(f, "{}({})\n", indent, err),
             Ok(packet) => write!(f, "{}{}\n", indent, packet)
         }
@@ -259,7 +277,7 @@ mod test {
 
     #[test]
     fn test_deconstruct() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         assert_eq!(packet.src_port(), 48896);
         assert_eq!(packet.dst_port(), 53);
         assert_eq!(packet.len(), 12);
@@ -271,7 +289,7 @@ mod test {
     #[test]
     fn test_construct() {
         let mut bytes = vec![0; 12];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         packet.set_src_port(48896);
         packet.set_dst_port(53);
         packet.set_len(12);
@@ -291,7 +309,7 @@ mod test {
 
     #[test]
     fn test_parse() {
-        let packet = Packet::new(&PACKET_BYTES[..]).unwrap();
+        let packet = Packet::new(&PACKET_BYTES[..]);
         let repr = Repr::parse(&packet, &SRC_ADDR.into(), &DST_ADDR.into()).unwrap();
         assert_eq!(repr, packet_repr());
     }
@@ -300,7 +318,7 @@ mod test {
     fn test_emit() {
         let repr = packet_repr();
         let mut bytes = vec![0; repr.buffer_len()];
-        let mut packet = Packet::new(&mut bytes).unwrap();
+        let mut packet = Packet::new(&mut bytes);
         repr.emit(&mut packet, &SRC_ADDR.into(), &DST_ADDR.into());
         assert_eq!(&packet.into_inner()[..], &PACKET_BYTES[..]);
     }