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

Return error instead of panics on invalid ASN.1 #9

Merged
merged 4 commits into from
Mar 29, 2019
Merged

Conversation

Flakebi
Copy link
Contributor

@Flakebi Flakebi commented Mar 29, 2019

Hi,
I use this library to find out if something is ASN.1 or not, unfortunately it often panics on invalid input.
To find all panics I added a cargo-fuzz project and fixed all the panics it found.
With the final version, the fuzzer did not find any panic for half an hour.

Unfortunately this pr currently includes a breaking change because it adds error variants. So I’m not sure if you want to implement this someway else, e.g. return some of the existing errors instead.

Edit: I found this commit which also adds a + Error bound to the error types.

@acw
Copy link
Owner

acw commented Mar 29, 2019

This is awesome, and I'm always happy to add more (and more helpful) error variants. Let me check a couple things and then I'll merge it.

@Flakebi
Copy link
Contributor Author

Flakebi commented Mar 29, 2019

Thanks!

One thing I forgot to mention: The GeneralizedTime parser previously inserted the dot at position 15, 14 should be the right position :)

@acw
Copy link
Owner

acw commented Mar 29, 2019

Yes, I saw that and that was one of the things I wanted to look at (and maybe write a test case for). :)

@acw acw merged commit 800f165 into acw:master Mar 29, 2019
@acw
Copy link
Owner

acw commented Mar 29, 2019

This version has been pushed as 0.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants