From f316e50dad5129f882f92db0b24e3ca6d75ab776 Mon Sep 17 00:00:00 2001 From: Koen Date: Tue, 28 Apr 2026 15:01:50 +0200 Subject: [PATCH 1/6] Error out on non-zero unused bit in DER encoding --- src/string/bit.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/string/bit.rs b/src/string/bit.rs index 1738a09..2efd44a 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -182,9 +182,15 @@ impl BitString { } let bits = inner.take_all()?; - // Strictly speaking, we should also check if the unused bits - // in the last octet are zero. - + if unused > 0 && inner.mode() == Mode::Der { + let mask = (1u8 << unused) - 1; + if bits[bits.len() - 1] & mask != 0 { + return Err(content.content_err( + "non-zero unused bits in DER bit string" + )); + } + } + Ok(BitString { unused, bits }) } decode::Content::Constructed(ref inner) => { From 9312edd8cf804709a48a5fe8724d190b848a5be3 Mon Sep 17 00:00:00 2001 From: Koen Date: Mon, 11 May 2026 14:02:51 +0200 Subject: [PATCH 2/6] Add test, make skip the same as from_content --- src/string/bit.rs | 44 ++++++++------------------------------------ 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/src/string/bit.rs b/src/string/bit.rs index 2c72607..e1415bc 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -182,7 +182,10 @@ impl BitString { } let bits = inner.take_all()?; - if unused > 0 && inner.mode() == Mode::Der { + if unused > 0 && + (inner.mode() == Mode::Der || inner.mode() == Mode::Cer) && + !bits.is_empty() + { let mask = (1u8 << unused) - 1; if bits[bits.len() - 1] & mask != 0 { return Err(content.content_err( @@ -212,40 +215,8 @@ impl BitString { pub fn skip_content( content: &mut decode::Content ) -> Result<(), DecodeError> { - match *content { - decode::Content::Primitive(ref mut inner) => { - if inner.mode() == Mode::Cer && inner.remaining() > 1000 { - return Err(content.content_err( - "long bit string component in CER mode" - )) - } - let unused = inner.take_u8()?; - if unused > 7 { - return Err(content.content_err( - "invalid bit string with large initial octet" - )); - } - if inner.remaining() == 0 && unused > 0 { - return Err(content.content_err( - "invalid bit string \ - (non-zero initial with empty bits)" - )); - } - inner.skip_all() - } - decode::Content::Constructed(ref inner) => { - if inner.mode() == Mode::Der { - Err(content.content_err( - "constructed bit string in DER mode" - )) - } - else { - Err(content.content_err( - "constructed bit string not implemented" - )) - } - } - } + Self::from_content(content)?; + return Ok(()); } /// Returns a value encoder that encodes a bytes slice as an octet string. @@ -387,7 +358,8 @@ mod test { check(b"\x03\x01\x00", Some((0, b""))); check(b"\x03\x07\x12deadb\xd0", None); check(b"\x03\x01\x04", None); - check(b"\x03\x00", None); + check(b"\x03\x02\x03ff", None); + check(b"\x03", None); } #[test] From a280ab423ce2d9c3ffcc760b6e8cc25a1d0a2ee6 Mon Sep 17 00:00:00 2001 From: Koen Date: Mon, 11 May 2026 14:04:08 +0200 Subject: [PATCH 3/6] Restore accidentally removed test --- src/string/bit.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/string/bit.rs b/src/string/bit.rs index e1415bc..2c01684 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -359,6 +359,7 @@ mod test { check(b"\x03\x07\x12deadb\xd0", None); check(b"\x03\x01\x04", None); check(b"\x03\x02\x03ff", None); + check(b"\x03\x00", None); check(b"\x03", None); } From 31080f8132ddb3f1b50f73a334e8e24ed1e71846 Mon Sep 17 00:00:00 2001 From: Koen Date: Mon, 11 May 2026 14:05:06 +0200 Subject: [PATCH 4/6] Make Clippy happy --- src/string/bit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/string/bit.rs b/src/string/bit.rs index 2c01684..f5e2268 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -216,7 +216,7 @@ impl BitString { content: &mut decode::Content ) -> Result<(), DecodeError> { Self::from_content(content)?; - return Ok(()); + Ok(()) } /// Returns a value encoder that encodes a bytes slice as an octet string. From 25fdd42acfe30b0f8e669e1f362d9ebbf7f41955 Mon Sep 17 00:00:00 2001 From: Koen Date: Mon, 11 May 2026 15:34:10 +0200 Subject: [PATCH 5/6] Process feedback --- src/string/bit.rs | 71 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/src/string/bit.rs b/src/string/bit.rs index f5e2268..5a715ac 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -183,14 +183,15 @@ impl BitString { let bits = inner.take_all()?; if unused > 0 && - (inner.mode() == Mode::Der || inner.mode() == Mode::Cer) && - !bits.is_empty() + (inner.mode() == Mode::Der || inner.mode() == Mode::Cer) { - let mask = (1u8 << unused) - 1; - if bits[bits.len() - 1] & mask != 0 { - return Err(content.content_err( - "non-zero unused bits in DER bit string" - )); + if let Some(last) = bits.last() { + let mask = (1u8 << unused) - 1; + if last & mask != 0 { + return Err(content.content_err( + "non-zero unused bits in DER bit string" + )); + } } } @@ -215,8 +216,60 @@ impl BitString { pub fn skip_content( content: &mut decode::Content ) -> Result<(), DecodeError> { - Self::from_content(content)?; - Ok(()) + match *content { + decode::Content::Primitive(ref mut inner) => { + if inner.mode() == Mode::Cer && inner.remaining() > 1000 { + return Err(content.content_err( + "long bit string component in CER mode" + )) + } + let unused = inner.take_u8()?; + if unused > 7 { + return Err(content.content_err( + "invalid bit string with large initial octet" + )); + } + if inner.remaining() == 0 && unused > 0 { + return Err(content.content_err( + "invalid bit string \ + (non-zero initial with empty bits)" + )); + } + + if inner.remaining() > 0 { + inner.skip(inner.remaining() - 1)?; + } + + let bits = inner.take_all()?; + + if unused > 0 && + (inner.mode() == Mode::Der || inner.mode() == Mode::Cer) + { + if let Some(last) = bits.last() { + let mask = (1u8 << unused) - 1; + if last & mask != 0 { + return Err(content.content_err( + "non-zero unused bits in DER bit string" + )); + } + } + } + + Ok(()) + } + decode::Content::Constructed(ref inner) => { + if inner.mode() == Mode::Der { + Err(content.content_err( + "constructed bit string in DER mode" + )) + } + else { + Err(content.content_err( + "constructed bit string not implemented" + )) + } + } + } } /// Returns a value encoder that encodes a bytes slice as an octet string. From 413c2047a896b44666abe7c38fc3afe997669e85 Mon Sep 17 00:00:00 2001 From: Koen Date: Mon, 11 May 2026 15:55:34 +0200 Subject: [PATCH 6/6] Add better test cases --- src/string/bit.rs | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/src/string/bit.rs b/src/string/bit.rs index 5a715ac..bec7252 100644 --- a/src/string/bit.rs +++ b/src/string/bit.rs @@ -229,23 +229,26 @@ impl BitString { "invalid bit string with large initial octet" )); } - if inner.remaining() == 0 && unused > 0 { - return Err(content.content_err( - "invalid bit string \ - (non-zero initial with empty bits)" - )); - } - - if inner.remaining() > 0 { - inner.skip(inner.remaining() - 1)?; - } - - let bits = inner.take_all()?; + if inner.remaining() == 0 { + if unused > 0 { + return Err(content.content_err( + "invalid bit string \ + (non-zero initial with empty bits)" + )); + } + } else { + if let Some(remaining) = inner.remaining().checked_sub(1) { + inner.skip(remaining)?; + } - if unused > 0 && - (inner.mode() == Mode::Der || inner.mode() == Mode::Cer) - { - if let Some(last) = bits.last() { + let last = inner.take_u8()?; + + if unused > 0 && + ( + inner.mode() == Mode::Der || + inner.mode() == Mode::Cer + ) + { let mask = (1u8 << unused) - 1; if last & mask != 0 { return Err(content.content_err( @@ -407,11 +410,15 @@ mod test { } } + check(b"\x03\x07\x00deadb\xdf", Some((0, b"deadb\xdf"))); check(b"\x03\x07\x04deadb\xd0", Some((4, b"deadb\xd0"))); + check(b"\x03\x07\x07deadb\x80", Some((7, b"deadb\x80"))); check(b"\x03\x01\x00", Some((0, b""))); - check(b"\x03\x07\x12deadb\xd0", None); + check(b"\x03\x07\x08deadb\xdf", None); + check(b"\x03\x07\xffdeadb\xdf", None); check(b"\x03\x01\x04", None); - check(b"\x03\x02\x03ff", None); + check(b"\x03\x02\x03\xff", None); + check(b"\x03\x03\x03\x00\xff", None); check(b"\x03\x00", None); check(b"\x03", None); }