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.

Sign up for GitHub

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

Jump to bottom

Further fuzzing efforts and returning error codes #15

Merged
merged 11 commits into from
Dec 6, 2022
Merged

Further fuzzing efforts and returning error codes #15

merged 11 commits into from
Dec 6, 2022

Conversation

szszszsz
Copy link
Member

@szszszsz szszszsz commented Dec 5, 2022 •
edited
Loading

Further fuzzing support and fixes found during its executions.

  • Corpus generated with the pynitrokey API tests. Each input file contains multiple commands. No new errors found after 2 hours minutes, with 12 jobs set.
  • Now error codes are returned over the CTAPHID transport as well, allowing improved error handling client side.

Future work / to discuss:

  • might be worth to run it longer, or in CI
  • extend the corpus to have more commands - Select, Validate
  • disable not tested commands (CalculateAll)
  • add coverage report generation
  • extend the corpus to run multiple commands at a time
  • dockerize/reuse fuzz setup
  • more efficient data splitting for the fuzzer - parse() function
  • remove ResultT, and use Result with a default type

fuzz_coverage.zip

Fixes #8
Connected #11

@szszszsz szszszsz added the enhancement New feature or request label Dec 5, 2022
@szszszsz szszszsz mentioned this pull request Dec 5, 2022
Merged
9 tasks
sosthene-nitrokey
sosthene-nitrokey requested changes Dec 6, 2022
View reviewed changes
fuzz/Makefile Outdated Show resolved Hide resolved
fuzz/fuzz_targets/fuzz_target_1.rs Outdated Show resolved Hide resolved
sosthene-nitrokey
sosthene-nitrokey requested changes Dec 6, 2022
View reviewed changes
fuzz/Makefile Outdated Show resolved Hide resolved
fuzz/fuzz_targets/fuzz_target_1.rs Outdated Show resolved Hide resolved
fuzz/fuzz_targets/fuzz_target_1.rs Outdated Show resolved Hide resolved
@szszszsz
Add corpus files as an archive
20e232d 
For a quick fuzzing start, or to use in the CI.
Generated from the OtpApp tests attached to pynitrokey.
@szszszsz
Helper: move coverage dir to /tmp
f573833
@szszszsz
Prefix response over CTAPHID with error codes
5dc5148 
For a better error handling. Requires updated client to use.
@szszszsz
Disable untested function: CalculateAll
f364212 
This function was not tested, and not used at all in the CLI implementation for pynitrokey,
hence disabling it until it would be useful.
@szszszsz
Make the fuzzing exection 10x faster
93b9bae
@szszszsz
Always ask for UP confirmation on important actions
3e86880 
Reset
Register
Clear and Set Password
@szszszsz
Refactor: make Result type generic, with default empty type
8553f17
@szszszsz
Document fuzzing process
daaeb16
@szszszsz szszszsz closed this in 5e4f990 Dec 6, 2022
@szszszsz szszszsz merged commit 5e4f990 into main Dec 6, 2022
@szszszsz szszszsz deleted the 8-fuzzing-2 branch December 6, 2022 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sosthene-nitrokey sosthene-nitrokey sosthene-nitrokey requested changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stability improvements
2 participants
@szszszsz @sosthene-nitrokey