Skip to content

Commit

Permalink
Fixed error with parsing SOA RRs in DNS
Browse files Browse the repository at this point in the history 
The variables i and remaining were being incremented before the rdata was
parsed. They were also directly accessed from within the match case for SOA.

Fixed packet::tests::parse_failing_packet_* to fail on error now.

Signed-off-by: Mohammad Aadil Shabier <aadilshabier1@gmail.com>
  • Loading branch information
@aadilshabier @gabhijit
aadilshabier authored and gabhijit committed Feb 28, 2024
1 parent 07ae387 commit 9b46bc1
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 45 deletions.
72 changes: 40 additions & 32 deletions src/layers/dns.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl fmt::Display for DNSName {
name_parts.push(out_str);
i += x + 1;
}
};
}
let name = name_parts.join(".");
write!(f, "{}", name)
}
Expand Down Expand Up @@ -95,6 +95,7 @@ pub struct DNSResRecord {
class: u16,
ttl: u32,
rdlength: u16,
#[serde(flatten)]
rdata: DNSRecordData,
}

Expand Down Expand Up @@ -245,54 +246,58 @@ impl DNS {
}
let rdata_buffer = &bytes[offset..offset + rdlength as usize];

i += 10 + rdlength as usize;
remaining -= 10 + rdlength as usize;
i += 10;
remaining -= 10;

let rdata = match type_ {
1 => DNSRecordData::A(rdata_buffer.try_into().unwrap()), /* A */
28 => DNSRecordData::AAAA(rdata_buffer.try_into().unwrap()), /* AAAA */
2 | 3 | 4 | 5 | 7 | 8 | 9 => {
let (name, _) = Self::dns_name_from_bytes(bytes, i, remaining)?;
let (name, _) = Self::dns_name_from_bytes(bytes, offset, remaining)?;
DNSRecordData::CNAME(name)
}
6 => {
6 => /* SOA */ {
// FIXME: into an inline function?
let (mname, consumed) = Self::dns_name_from_bytes(bytes, i, remaining)?;
i += consumed;
let mut offset = offset;
let mut remaining = remaining;
let (mname, consumed) = Self::dns_name_from_bytes(bytes, offset, remaining)?;
offset += consumed;
remaining -= consumed;
let (rname, consumed) = Self::dns_name_from_bytes(bytes, i, remaining)?;
i += consumed;
let (rname, consumed) = Self::dns_name_from_bytes(bytes, offset, remaining)?;
offset += consumed;
remaining -= consumed;
if remaining < 20 {
return Err(Error::TooShort {
required: 20,
available: remaining,
data: hex::encode(&bytes[i..]),
data: hex::encode(&bytes[offset..]),
});
}
// serial, refresh, retry, expire, minimum
let serial = (bytes[i] as u32) << 24
| (bytes[i + 1] as u32) << 16
| (bytes[i + 2] as u32) << 8
| (bytes[i + 3] as u32);
let refresh = (bytes[i + 4] as u32) << 24
| (bytes[i + 5] as u32) << 16
| (bytes[i + 6] as u32) << 8
| (bytes[i + 7] as u32);
let retry = (bytes[i + 8] as u32) << 24
| (bytes[i + 9] as u32) << 16
| (bytes[i + 10] as u32) << 8
| (bytes[i + 11] as u32);
let expire = (bytes[i + 12] as u32) << 24
| (bytes[i + 13] as u32) << 16
| (bytes[i + 14] as u32) << 8
| (bytes[i + 15] as u32);
let minimum = (bytes[i + 16] as u32) << 24
| (bytes[i + 17] as u32) << 16
| (bytes[i + 18] as u32) << 8
| (bytes[i + 19] as u32);

i += 20;
let serial = (bytes[offset] as u32) << 24
| (bytes[offset + 1] as u32) << 16
| (bytes[offset + 2] as u32) << 8
| (bytes[offset + 3] as u32);
let refresh = (bytes[offset + 4] as u32) << 24
| (bytes[offset + 5] as u32) << 16
| (bytes[offset + 6] as u32) << 8
| (bytes[offset + 7] as u32);
let retry = (bytes[offset + 8] as u32) << 24
| (bytes[offset + 9] as u32) << 16
| (bytes[offset + 10] as u32) << 8
| (bytes[offset + 11] as u32);
let expire = (bytes[offset + 12] as u32) << 24
| (bytes[offset + 13] as u32) << 16
| (bytes[offset + 14] as u32) << 8
| (bytes[offset + 15] as u32);
let minimum = (bytes[offset + 16] as u32) << 24
| (bytes[offset + 17] as u32) << 16
| (bytes[offset + 18] as u32) << 8
| (bytes[offset + 19] as u32);

// offset += 20;
// remaining -= 20;

DNSRecordData::SOA(DNSSOA {
mname,
rname,
Expand All @@ -303,8 +308,11 @@ impl DNS {
minimum,
})
}
// 41 => unimplemented!("Parse OPT"),
_ => DNSRecordData::NULL(rdata_buffer.into()),
};
i += rdlength as usize;
// remaining -= rdlength as usize;

Ok((
DNSResRecord {
Expand Down
23 changes: 10 additions & 13 deletions src/packet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,9 @@ impl Packet {
log::debug!("L2 in packet: {}", l2.short_name());

let mut current_layer = l2;
let mut res: (Option<Box<dyn Layer + Send>>, usize);
let mut start = 0;
loop {
res = current_layer.decode_bytes(&bytes[start..])?;
let res = current_layer.decode_bytes(&bytes[start..])?;

if res.0.is_none() {
p.layers.push(current_layer);
Expand Down Expand Up @@ -315,31 +314,29 @@ mod tests {
fn parse_failing_packet() {
use crate::layers;
let _ = layers::register_defaults();
let failing = hex::decode("387a0ec884c6001a9adead05080045000130eed7000037118469ca58835bc0a801200035c8d1011c169cd65e81800001000100040007046368617406676f6f676c6503636f6d00001c0001c00c001c00010000012a0010240468004002081a000000000000200ec011000200010001f0f20006036e7334c011c011000200010001f0f20006036e7331c011c011000200010001f0f20006036e7332c011c011000200010001f0f20006036e7333c011c05b000100010001fe660004d8ef200ac06d000100010001f1ce0004d8ef220ac07f0001000100021aad0004d8ef240ac05b001c00010002c4e400102001486048020032000000000000000ac06d001c00010001f1ce00102001486048020034000000000000000ac07f001c000100050db900102001486048020036000000000000000a0000291000000000000000").unwrap();
let failing = Packet::from_bytes(&failing, ENCAP_TYPE_ETH);
let should_not_fail = hex::decode("387a0ec884c6001a9adead05080045000130eed7000037118469ca58835bc0a801200035c8d1011c169cd65e81800001000100040007046368617406676f6f676c6503636f6d00001c0001c00c001c00010000012a0010240468004002081a000000000000200ec011000200010001f0f20006036e7334c011c011000200010001f0f20006036e7331c011c011000200010001f0f20006036e7332c011c011000200010001f0f20006036e7333c011c05b000100010001fe660004d8ef200ac06d000100010001f1ce0004d8ef220ac07f0001000100021aad0004d8ef240ac05b001c00010002c4e400102001486048020032000000000000000ac06d001c00010001f1ce00102001486048020034000000000000000ac07f001c000100050db900102001486048020036000000000000000a0000291000000000000000").unwrap();
let should_not_fail = Packet::from_bytes(&should_not_fail, ENCAP_TYPE_ETH);

assert!(failing.is_err(), "{:?}", failing.ok());
assert!(should_not_fail.is_ok(), "{:?}", should_not_fail.err());
}

#[ignore]
#[test]
fn parse_failing_packet_2() {
use crate::layers;
let _ = layers::register_defaults();
let failing = hex::decode("387a0ec884c6001a9adead05080045000163902e00003d11b84c7448fdfec0a8012000359cca014f1eabbbd3818000010001000400090373736c076773746174696303636f6d00001c0001c00c001c0001000000fc001024046800400908130000000000002003c0100002000100016de4000d036e733406676f6f676c65c018c0100002000100016de40006036e7331c04dc0100002000100016de40006036e7332c04dc0100002000100016de40006036e7333c04dc0620001000100016df50004d8ef200ac0740001000100016df50004d8ef220ac0860001000100016df50004d8ef240ac0490001000100016df50004d8ef260ac062001c000100016df500102001486048020032000000000000000ac074001c000100016df500102001486048020034000000000000000ac086001c000100016df500102001486048020036000000000000000ac049001c000100016df500102001486048020038000000000000000a0000291000000000000000").unwrap();
let failing = Packet::from_bytes(&failing, ENCAP_TYPE_ETH);
let should_not_fail = hex::decode("387a0ec884c6001a9adead05080045000163902e00003d11b84c7448fdfec0a8012000359cca014f1eabbbd3818000010001000400090373736c076773746174696303636f6d00001c0001c00c001c0001000000fc001024046800400908130000000000002003c0100002000100016de4000d036e733406676f6f676c65c018c0100002000100016de40006036e7331c04dc0100002000100016de40006036e7332c04dc0100002000100016de40006036e7333c04dc0620001000100016df50004d8ef200ac0740001000100016df50004d8ef220ac0860001000100016df50004d8ef240ac0490001000100016df50004d8ef260ac062001c000100016df500102001486048020032000000000000000ac074001c000100016df500102001486048020034000000000000000ac086001c000100016df500102001486048020036000000000000000ac049001c000100016df500102001486048020038000000000000000a0000291000000000000000").unwrap();
let should_not_fail = Packet::from_bytes(&should_not_fail, ENCAP_TYPE_ETH);

assert!(failing.is_ok(), "{:?}", failing.err());
assert!(should_not_fail.is_ok(), "{:?}", should_not_fail.err());
}

#[ignore]
#[test]
fn parse_failing_packet_3() {
use crate::layers;
let _ = layers::register_defaults();
let failing = hex::decode("00000304000600000000000000000800450000c01b0b400001115fec7f0000357f0000010035dd3600acfef3c8218180000100020004000108697076346f6e6c7904617270610000010001c00c00010001000017210004c00000aac00c00010001000017210004c00000abc00c0002000100001721001401620c69616e612d73657276657273036e657400c00c000200010000172100040161c04dc00c000200010000172100040163c04dc00c0002000100001721000e026e73056963616e6e036f726700000029ffd6000000000000").unwrap();
let failing = Packet::from_bytes(&failing, ENCAP_TYPE_LINUX_SLL);
let should_not_fail = hex::decode("00000304000600000000000000000800450000c01b0b400001115fec7f0000357f0000010035dd3600acfef3c8218180000100020004000108697076346f6e6c7904617270610000010001c00c00010001000017210004c00000aac00c00010001000017210004c00000abc00c0002000100001721001401620c69616e612d73657276657273036e657400c00c000200010000172100040161c04dc00c000200010000172100040163c04dc00c0002000100001721000e026e73056963616e6e036f726700000029ffd6000000000000").unwrap();
let should_not_fail = Packet::from_bytes(&should_not_fail, ENCAP_TYPE_LINUX_SLL);

assert!(failing.is_err(), "{:?}", failing.ok());
assert!(should_not_fail.is_ok(), "{:?}", should_not_fail.err());
}
}

0 comments on commit 9b46bc1

Please sign in to comment.