Browse Source

Throw an error for non-zero LOC record versions

This is a tricky one, because we have to assume that a version 1 will be added at some point in time (and write the error message appropriately), even though (I'm pretty sure that) no such version currently exists.
Benjamin Sago 4 years ago
parent
commit
b295a6c2dd
3 changed files with 53 additions and 6 deletions
  1. 38 6
      dns/src/record/loc.rs
  2. 12 0
      dns/src/wire.rs
  3. 3 0
      src/output.rs

+ 38 - 6
dns/src/record/loc.rs

@@ -52,15 +52,18 @@ impl Wire for LOC {
 
     #[cfg_attr(all(test, feature = "with_mutagen"), ::mutagen::mutate)]
     fn read(stated_length: u16, c: &mut Cursor<&[u8]>) -> Result<Self, WireError> {
-        if stated_length != 16 {
-            return Err(WireError::WrongRecordLength { stated_length, mandated_length: 16 });
-        }
-
         let version = c.read_u8()?;
         trace!("Parsed version -> {:?}", version);
 
         if version != 0 {
-            warn!("LOC version is not 0");
+            return Err(WireError::WrongVersion {
+                stated_version: version,
+                maximum_supported_version: 0,
+            });
+        }
+
+        if stated_length != 16 {
+            return Err(WireError::WrongRecordLength { stated_length, mandated_length: 16 });
         }
 
         let size_bits = c.read_u8()?;
@@ -98,6 +101,7 @@ impl fmt::Display for Size {
     }
 }
 
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -136,10 +140,38 @@ mod test {
                    Err(WireError::WrongRecordLength { stated_length: 2, mandated_length: 16 }));
     }
 
+    #[test]
+    fn record_too_long() {
+        let buf = &[
+            0x00,  // version
+            0x32,  // size,
+            0x00,  // horizontal precision
+            0x00,  // vertical precision
+            0x8b, 0x0d, 0x2c, 0x8c,  // latitude
+            0x7f, 0xf8, 0xfc, 0xa5,  // longitude
+            0x00, 0x98, 0x96, 0x80,  // altitude
+            0x12, 0x34, 0x56,  // some other stuff
+        ];
+
+        assert_eq!(LOC::read(buf.len() as _, &mut Cursor::new(buf)),
+                   Err(WireError::WrongRecordLength { stated_length: 19, mandated_length: 16 }));
+    }
+
+    #[test]
+    fn more_recent_version() {
+        let buf = &[
+            0x80,  // version
+            0x12, 0x34, 0x56,  // some data in an unknown format
+        ];
+
+        assert_eq!(LOC::read(buf.len() as _, &mut Cursor::new(buf)),
+                   Err(WireError::WrongVersion { stated_version: 128, maximum_supported_version: 0 }));
+    }
+
     #[test]
     fn record_empty() {
         assert_eq!(LOC::read(0, &mut Cursor::new(&[])),
-                   Err(WireError::WrongRecordLength { stated_length: 0, mandated_length: 16 }));
+                   Err(WireError::IO));
     }
 
     #[test]

+ 12 - 0
dns/src/wire.rs

@@ -443,6 +443,18 @@ pub enum WireError {
     /// When the data contained a string with a pointer to an index outside of
     /// the packet. Contains the invalid index.
     OutOfBounds(u16),
+
+    /// When a record in the packet contained a version field that specifies
+    /// the format of its remaining fields, but this version is too recent to
+    /// be supported, so we cannot parse it.
+    WrongVersion {
+
+        /// The version of the record layout, as specified in the packet
+        stated_version: u8,
+
+        /// The maximum version that this version of dog supports.
+        maximum_supported_version: u8,
+    }
 }
 
 impl From<io::Error> for WireError {

+ 3 - 0
src/output.rs

@@ -528,5 +528,8 @@ fn wire_error_message(error: WireError) -> String {
         WireError::OutOfBounds(index) => {
             format!("Malformed packet: out of bounds ({})", index)
         }
+        WireError::WrongVersion { stated_version, maximum_supported_version } => {
+            format!("Malformed packet: record specifies version {}, expected up to {}", stated_version, maximum_supported_version)
+        }
     }
 }