diff --git a/ul/fuzz/Cargo.lock b/ul/fuzz/Cargo.lock index 3baf218ff..13da7de5b 100644 --- a/ul/fuzz/Cargo.lock +++ b/ul/fuzz/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "android-tzdata" @@ -93,7 +93,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "dicom-core" -version = "0.7.1" +version = "0.8.0" dependencies = [ "chrono", "itertools", @@ -105,7 +105,7 @@ dependencies = [ [[package]] name = "dicom-dictionary-std" -version = "0.7.0" +version = "0.8.0" dependencies = [ "dicom-core", "once_cell", @@ -113,7 +113,7 @@ dependencies = [ [[package]] name = "dicom-encoding" -version = "0.7.1" +version = "0.8.0" dependencies = [ "byteordered", "dicom-core", @@ -124,7 +124,7 @@ dependencies = [ [[package]] name = "dicom-transfer-syntax-registry" -version = "0.7.1" +version = "0.8.0" dependencies = [ "byteordered", "dicom-core", @@ -135,7 +135,7 @@ dependencies = [ [[package]] name = "dicom-ul" -version = "0.7.1" +version = "0.8.0" dependencies = [ "byteordered", "bytes", diff --git a/ul/fuzz/fuzz.sh b/ul/fuzz/fuzz.sh new file mode 100755 index 000000000..6b3724ff9 --- /dev/null +++ b/ul/fuzz/fuzz.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env sh + +env ASAN_OPTIONS=allocator_may_return_null=1:max_allocation_size_mb=40 cargo +nightly fuzz run pdu_roundtrip diff --git a/ul/fuzz/fuzz_targets/pdu_roundtrip.rs b/ul/fuzz/fuzz_targets/pdu_roundtrip.rs index 7a271417a..f3908fe38 100644 --- a/ul/fuzz/fuzz_targets/pdu_roundtrip.rs +++ b/ul/fuzz/fuzz_targets/pdu_roundtrip.rs @@ -10,7 +10,9 @@ fuzz_target!(|data: (u32, bool, &[u8])| { fn fuzz(maxlen: u32, strict: bool, mut data: &[u8]) -> Result<(), Box> { // deserialize random bytes - let pdu = dicom_ul::pdu::read_pdu(&mut data, maxlen, strict)?; + let Some(pdu) = dicom_ul::pdu::read_pdu(&mut data, maxlen, strict)? else { + return Ok(()); + }; // serialize pdu back to bytes let mut bytes = Vec::new(); @@ -18,7 +20,8 @@ fn fuzz(maxlen: u32, strict: bool, mut data: &[u8]) -> Result<(), Box // deserialize back to pdu let pdu2 = dicom_ul::pdu::read_pdu(&mut bytes.as_slice(), maxlen, strict) - .expect("serialized pdu should always deserialize"); + .expect("serialized pdu should always deserialize") + .expect("serialized pdu should exist"); // assert equivalence assert_eq!( diff --git a/ul/src/pdu/reader.rs b/ul/src/pdu/reader.rs index aa2b1641a..e3e6999b4 100644 --- a/ul/src/pdu/reader.rs +++ b/ul/src/pdu/reader.rs @@ -94,16 +94,13 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // Version 1 and shall be identified with bit 0 set. A receiver of this PDU // implementing only this version of the DICOM UL protocol shall only test that bit // 0 is set. - if bytes.remaining() < 2 { + if bytes.remaining() < 2 + 2 + 16 + 16 + 32 { return Ok(None); } let protocol_version = bytes.get_u16(); // 9-10 - Reserved - This reserved field shall be sent with a value 0000H but not // tested to this value when received. - if bytes.remaining() < 2 { - return Ok(None); - } bytes.get_u16(); // 11-26 - Called-AE-title - Destination DICOM Application Name. It shall be encoded @@ -157,7 +154,7 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< return InvalidPduVariableSnafu { var_item }.fail(); } None => { - println!("PDU variable none"); + tracing::debug!("PDU variable none"); return Ok(None); } } @@ -185,16 +182,13 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // Version 1 and shall be identified with bit 0 set. A receiver of this PDU // implementing only this version of the DICOM UL protocol shall only test that bit // 0 is set. - if bytes.remaining() < 2 { + if bytes.remaining() < 2 + 2 + 16 + 16 + 32 { return Ok(None); } let protocol_version = bytes.get_u16(); // 9-10 - Reserved - This reserved field shall be sent with a value 0000H but not // tested to this value when received. - if bytes.remaining() < 2 { - return Ok(None); - } bytes.get_u16(); // 11-26 - Reserved - This reserved field shall be sent with a value identical to @@ -263,7 +257,7 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // 7 - Reserved - This reserved field shall be sent with a value 00H but not tested to // this value when received. - if bytes.remaining() < 1 { + if bytes.remaining() < 1 + 1 + 2 { return Ok(None); } bytes.get_u8(); @@ -272,9 +266,6 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // binary number. One of the following values shall be used: // 1 - rejected-permanent // 2 - rejected-transient - if bytes.remaining() < 1 { - return Ok(None); - } let result = AssociationRJResult::from(bytes.get_u8()) .context(InvalidRejectSourceOrReasonSnafu)?; @@ -299,9 +290,6 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // 1 - temporary-congestio // 2 - local-limit-exceeded // 3-7 - reserved - if bytes.remaining() < 2 { - return Ok(None); - } let source = AssociationRJSource::from(bytes.get_u8(), bytes.get_u8()) .context(InvalidRejectSourceOrReasonSnafu)?; @@ -320,7 +308,7 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // 1-4 - Item-length - This Item-length shall be the number of bytes from the first // byte of the following field to the last byte of the Presentation-data-value // field. It shall be encoded as an unsigned binary number. - if bytes.remaining() < 4 { + if bytes.remaining() < 4 + 1 + 1 { return Ok(None); } let item_length = bytes.get_u32(); @@ -335,9 +323,6 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // 5 - Presentation-context-ID - Presentation-context-ID values shall be odd // integers between 1 and 255, encoded as an unsigned binary number. For a complete // description of the use of this field see Section 7.1.1.13. - if bytes.remaining() < 1 { - return Ok(None); - } let presentation_context_id = bytes.get_u8(); // 6-xxx - Presentation-data-value - This Presentation-data-value field shall @@ -353,9 +338,6 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // following fragment shall contain the last fragment of a Message Data Set or of a // Message Command. If bit 1 is set to 0, the following fragment // does not contain the last fragment of a Message Data Set or of a Message Command. - if bytes.remaining() < 1 { - return Ok(None); - } let header = bytes.get_u8(); let value_type = if header & 0x01 > 0 { @@ -382,6 +364,9 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // 7-10 - Reserved - This reserved field shall be sent with a value 00000000H but not // tested to this value when received. + if bytes.remaining() < 4 { + return Ok(None); + } bytes.advance(4); Ok(Some(Pdu::ReleaseRQ)) @@ -405,7 +390,7 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // this value when received. // 8 - Reserved - This reserved field shall be sent with a value 00H but not tested to // this value when received. - if bytes.remaining() < 2 { + if bytes.remaining() < 2 + 2 { return Ok(None); } let _ = bytes.copy_to_bytes(2); @@ -424,9 +409,6 @@ pub fn read_pdu(mut buf: impl Buf, max_pdu_length: u32, strict: bool) -> Result< // - 4 - unrecognized-PDU parameter // - 5 - unexpected-PDU parameter // - 6 - invalid-PDU-parameter value - if bytes.remaining() < 2 { - return Ok(None); - } let source = AbortRQSource::from(bytes.get_u8(), bytes.get_u8()) .context(InvalidAbortSourceOrReasonSnafu)?;