Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Int64 overflow when parsing structure length #24

Closed
emillon opened this issue Mar 8, 2019 · 2 comments
Closed

Int64 overflow when parsing structure length #24

emillon opened this issue Mar 8, 2019 · 2 comments

Comments

@emillon
Copy link

emillon commented Mar 8, 2019

Hi,

As #23 this was found in a Wycheproof test case.

When parsing BER, the length of structures is computed on int64s, which can overflow.
The following case is accepted, while it should be rejected:

  anticase "structure length overflow"
  Asn.S.(sequence (single (required integer))) [
    [ (* sequence; length is encoded in next 9 bytes *)
      0x30; 0x89;
      (* len = 2**64 + 3 *)
      0x01; 0x00; 0x00; 0x00; 0x00; 0x00; 0x00; 0x00; 0x03;
      (* integer, len = 1 *)
      0x02; 0x01;
      (* value = 0 *)
      0x00;
    ]];

Note that the same starting with 0x30 0x89 0x00 should be accepted since it's a valid encoding of len=3.

Thanks!

@pqwy
Copy link
Contributor

pqwy commented Apr 18, 2019

This is handled there.

Unfortunately, 1,818,000,000 executions later, and I still cannot get AFL to detect this. I suspect that the guided fuzzing it's employing is a little blind to overflows -- as long as they are memory-safe -- as they don't change the control flow. 😢

hannesm added a commit to hannesm/opam-repository that referenced this issue Jan 28, 2020
CHANGES:

* disallow various constructs as suggested by ITU-T Rec X.690 (by @pqwy)
  * redundant OID component forms (X.690 8.20.2)
  * redundant integer forms (X.690 8.3.2)
  * empty integer (X.690 8.3.1, reported in mirleft/ocaml-asn1-combinators#23 by @emillon)
  * constructed strings in DER
* deeper implict -> explicit over choice (follow-up to v0.2.0 entry, by @pqwy)
* handle long-form length overflow (reported in mirleft/ocaml-asn1-combinators#24 by @emillon, fixed by @pqwy)
* disallow primitive with indefinite length (introduced in the bugfix above,
  reported by @emillon, fixed in mirleft/ocaml-asn1-combinators#32 by @hannesm)
* disallow nonsensical bitstring unused values (X690 8.6.2, reported in mirleft/ocaml-asn1-combinators#26
  by @NathanReb, fixed by @pqwy)
* fix non-continuous bit_string_flags (X680 22.6, reported in mirleft/ocaml-asn1-combinators#25 by @wiml,
  fixed by @pqwy)
* use Alcotest instead of oUnit for unit tests (by @pqwy)
* use dune as build system (by @pqwy, superseeds mirleft/ocaml-asn1-combinators#22)
* use bigarray-compat (mirleft/ocaml-asn1-combinators#27 by @TheLortex) and stdlib-shims (mirleft/ocaml-asn1-combinators#29 by @XVilka)
* raise lower bound to OCaml 4.05.0 (mirleft/ocaml-asn1-combinators#31 by @hannesm)
@hannesm
Copy link
Member

hannesm commented Jan 28, 2020

thanks, this is fixed and part of the 0.2.1 release. closing.

@hannesm hannesm closed this as completed Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants