rasn icon indicating copy to clipboard operation
rasn copied to clipboard

`GeneralizedTime` encodes unaccepted value.

Open 5225225 opened this issue 3 years ago • 1 comments

My fuzzer for this was

#![no_main]
use libfuzzer_sys::fuzz_target;

fuzz_target!(|data: &[u8]| {
    if let Ok(value) = rasn::der::decode::<rasn::types::Open>(data) {
        assert_eq!(value, rasn::der::decode(&rasn::der::encode(&value).unwrap()).unwrap());
    }
});

Not sure if this a valid round fuzz test, feel free to close as invalid if not.

Reproducer is

fn main() {
    let data = [
        24, 19, 43, 53, 49, 54, 49, 53, 32, 32, 48, 53, 50, 52, 48, 57, 52, 48, 50, 48, 90,
    ];

    let value = rasn::der::decode::<rasn::types::Open>(&data).expect("passes");

    rasn::der::decode::<rasn::types::Open>(&rasn::der::encode(&value).unwrap()).expect("fails");
}

Fails with thread 'main' panicked at 'fails: NoValidChoice { name: "Open" }', src/main.rs:7:81

5225225 avatar Dec 23 '21 16:12 5225225

Thank you for your issue! This seems maybe valid, it's definitely a difference at least, though I'm not sure why yet, and not entirely sure how to fix it.

The supplied data is a GeneralizedTime type encoded as +51615 0524094020Z, where once as it's decoded and re-encoded it's encoded +516150524094020Z (notice the missing space), which is then rejected by the decoder.

Here's the snippets of where this encoded/decoded.

https://github.com/XAMPPRocky/rasn/blob/441810cbaefddce2e2a6612acc73f2d5e1775707/src/ber/enc.rs#L340-L355

https://github.com/XAMPPRocky/rasn/blob/441810cbaefddce2e2a6612acc73f2d5e1775707/src/ber/de.rs#L290-L296

XAMPPRocky avatar Dec 23 '21 17:12 XAMPPRocky

I am trying to fix this, but it seems that the decoder ~~is much more "loose", than the spec allows for GeneralizedTime.~~

~~If we go with the spec, it should be easy to handle all variations, but if we compare to other implementations (asn1tools), they handle more cases. E.g. they accept both , comma and . dot, or missing seconds or minutes, or also +/- offsets in encoded values, while the BER should never produce such outputs.~~

Actually I think this applies only for CER and DER, but is missing from them. So I guess both are needed.

Nicceboy avatar Aug 24 '23 21:08 Nicceboy