-
Notifications
You must be signed in to change notification settings - Fork 19
Fuzzing framework #969
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
base: master
Are you sure you want to change the base?
Fuzzing framework #969
Conversation
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice iteration. Will test it but it looks fairly easy to use and documented enough for new commers.
Made a review on the CMake scripts.
0b29590
to
6a77b64
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #969 +/- ##
==========================================
+ Coverage 61.45% 62.08% +0.62%
==========================================
Files 13 13
Lines 1816 1817 +1
==========================================
+ Hits 1116 1128 +12
+ Misses 700 689 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
443e651
to
da64141
Compare
5549a40
to
3c70913
Compare
3d5b806
to
6d18e2a
Compare
dcd5b2f
to
67b7893
Compare
FYI, if you want to test the framework with an app, you can have a look at the boilerplate PR LedgerHQ/app-boilerplate#163 |
0dea985
to
9d7064f
Compare
fuzzing/macros/macros-flex.txt
Outdated
@@ -0,0 +1,104 @@ | |||
API_LEVEL=22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API_LEVEL value is already obsolete now.
How can we force the macros to be regenerated, at least on the PR workflows?
Moreover, we should probably find a way to auto-update the macro files regularly, to ensure they are always up-to-date, aligned with the surrent SDK
Maybe those files should not be stored under git, but generated at runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gustavo-Jodar did you leave this as an example? Or is there another use case?
This API_LEVEL should be present in the compile_commands.json
when running from a released SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To generate those files I extracted the macro definitions used when building app-boilerplate with the SDK, and it was left there like this as an example that should be changed in order to fuzz the sdk.
Which exposes the problem of having them obsolete, since this API_LEVEL was the one used before a new version of the sdk (interval between this review and my commit).
This process could be automated, since it is just a matter of compiling app-boilerplate with bear and using extract_macros.py
to make the files.
fuzzing/macros/macros.cmake
Outdated
execute_process( | ||
COMMAND sh -c "BOLOS_SDK=\${STAX_SDK} && TARGET=${TARGET_DEVICE} && CC=clang make list-defines" | ||
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/macros" | ||
OUTPUT_VARIABLE MACRO_OUTPUT | ||
OUTPUT_STRIP_TRAILING_WHITESPACE | ||
ERROR_QUIET | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's try to get as far as possible from platform specific commands.
Cmake has a few commands/functinos for such use cases.
Here we could try something like cmake env
execute_process( | |
COMMAND sh -c "BOLOS_SDK=\${STAX_SDK} && TARGET=${TARGET_DEVICE} && CC=clang make list-defines" | |
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/macros" | |
OUTPUT_VARIABLE MACRO_OUTPUT | |
OUTPUT_STRIP_TRAILING_WHITESPACE | |
ERROR_QUIET | |
) | |
execute_process( | |
COMMAND ${CMAKE_COMMAND} -E env | |
BOLOS_SDK=${STAX_SDK} | |
TARGET=${TARGET_DEVICE} | |
CC=clang | |
make list-defines | |
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/macros" | |
OUTPUT_VARIABLE MACRO_OUTPUT | |
OUTPUT_STRIP_TRAILING_WHITESPACE | |
ERROR_QUIET | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we specify TARGET then we should point to the currently in use sdk instead of STAX_SDK
b1792de
to
23860b7
Compare
Description
New fuzzing framework for Ledger SDK.
If you want to test the framework with an app, you can have a look at the boilerplate PR LedgerHQ/app-boilerplate#163
Changes include