LibCrypto: Restore cached tag when a failed ASN1 read is rolled back

Previously, the stack would be restored to its previous position but
the tag would retain its old value, leading to an inconsistent state.
This commit is contained in:
Tim Ledbetter 2026-06-05 12:55:20 +01:00 committed by Tim Ledbetter
parent a8d729ff9c
commit ed6aec8dfa
2 changed files with 32 additions and 0 deletions

View file

@ -157,16 +157,19 @@ public:
return Error::from_string_literal("ASN1::Decoder: Trying to drop using a decoder that is EOF");
auto previous_position = m_stack;
auto previous_tag = m_current_tag;
auto tag_or_error = peek();
if (tag_or_error.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return tag_or_error.release_error();
}
auto length_or_error = read_length();
if (length_or_error.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return length_or_error.release_error();
}
@ -175,6 +178,7 @@ public:
auto bytes_result = read_bytes(length);
if (bytes_result.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return bytes_result.release_error();
}
@ -192,16 +196,19 @@ public:
return Error::from_string_literal("ASN1::Decoder: Trying to read using a decoder that is EOF");
auto previous_position = m_stack;
auto previous_tag = m_current_tag;
auto tag_or_error = peek();
if (tag_or_error.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return tag_or_error.release_error();
}
auto length_or_error = read_length();
if (length_or_error.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return length_or_error.release_error();
}
@ -211,6 +218,7 @@ public:
auto value_or_error = read_value<ValueType>(class_override.value_or(tag.class_), kind_override.value_or(tag.kind), length);
if (value_or_error.is_error()) {
m_stack = move(previous_position);
m_current_tag = move(previous_tag);
return value_or_error.release_error();
}

View file

@ -238,3 +238,27 @@ TEST_CASE(test_encoder_constructed)
MUST(decoder.leave()); // Sequence
EXPECT(decoder.eof()); // no other data
}
TEST_CASE(test_decoder_restores_state_after_failed_read)
{
// A Boolean tag declaring a length of 5, but only 3 value bytes are present.
u8 const data[] { 0x01, 0x05, 0x00, 0x00, 0x00 };
Crypto::ASN1::Decoder decoder({ data, sizeof(data) });
EXPECT(decoder.read<bool>().is_error());
// The first failed read should leave the decoder state unchanged, so a second drop should have the same result.
EXPECT(decoder.read<bool>().is_error());
}
TEST_CASE(test_decoder_restores_state_after_failed_drop)
{
// A Boolean tag declaring a length of 5, but only 3 value bytes are present.
u8 const data[] { 0x01, 0x05, 0x00, 0x00, 0x00 };
Crypto::ASN1::Decoder decoder({ data, sizeof(data) });
EXPECT(decoder.drop().is_error());
// The first failed drop should leave the decoder state unchanged, so a second drop should have the same result.
EXPECT(decoder.drop().is_error());
}