diff --git a/.clang-format b/.clang-format index b4abfa06..761f2684 100644 --- a/.clang-format +++ b/.clang-format @@ -1,20 +1,23 @@ --- BasedOnStyle: Google IndentWidth: 4 ---- Language: Cpp ColumnLimit: 100 PointerAlignment: Right AlignAfterOpenBracket: Align AlignConsecutiveMacros: true -AllowAllParametersOfDeclarationOnNextLine: false SortIncludes: false SpaceAfterCStyleCast: true -AllowShortCaseLabelsOnASingleLine: false AllowAllArgumentsOnNextLine: false AllowAllParametersOfDeclarationOnNextLine: false -AllowShortBlocksOnASingleLine: Never -AllowShortFunctionsOnASingleLine: None BinPackArguments: false BinPackParameters: false ---- +InsertBraces: true +AllowShortBlocksOnASingleLine: Never +AllowShortCaseLabelsOnASingleLine: false +AllowShortEnumsOnASingleLine: false +AllowShortFunctionsOnASingleLine: None +AllowShortIfStatementsOnASingleLine: Never +AllowShortLambdasOnASingleLine: None +AllowShortLoopsOnASingleLine: false +... diff --git a/.github/workflows/build_and_functional_tests_baking.yml b/.github/workflows/build_and_functional_tests_baking.yml index a37f7e1d..eb2044eb 100644 --- a/.github/workflows/build_and_functional_tests_baking.yml +++ b/.github/workflows/build_and_functional_tests_baking.yml @@ -31,4 +31,3 @@ jobs: with: download_app_binaries_artifact: "compiled_app_binaries" test_dir: test/python - diff --git a/.github/workflows/cppcheck.yml b/.github/workflows/cppcheck.yml new file mode 100644 index 00000000..ab91e79a --- /dev/null +++ b/.github/workflows/cppcheck.yml @@ -0,0 +1,19 @@ +name: cppcheck +on: [pull_request] + +jobs: + cppchceck: + name: Cppcheck + runs-on: ubuntu-latest + steps: + - name: Setup + run: sudo apt-get install -y -q cppcheck + + - name: Checkout + uses: actions/checkout@v4 + with: + repository: ${{ github.repository }} + ref: ${{ github.ref }} + + - name: Check + run: cppcheck --addon=misra.json --quiet --error-exitcode=1 src/ diff --git a/.github/workflows/lint-workflow.yml b/.github/workflows/lint-workflow.yml index 6d07bc91..1c994b04 100644 --- a/.github/workflows/lint-workflow.yml +++ b/.github/workflows/lint-workflow.yml @@ -1,6 +1,17 @@ name: Code style check -on: [push, pull_request] +# Use: +# LedgerHQ/ledger-app-workflows/.github/workflows/reusable_lint.yml +# once their lint-checker has been updated + +on: + workflow_dispatch: + push: + branches: + - master + - main + - develop + pull_request: jobs: job_lint: @@ -12,8 +23,8 @@ jobs: uses: actions/checkout@v2 - name: Lint - uses: DoozyX/clang-format-lint-action@v0.11 + uses: DoozyX/clang-format-lint-action@v0.17 with: source: "./" extensions: "h,c" - clangFormatVersion: 10 + clangFormatVersion: 17 diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 00000000..236186c0 --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,16 @@ +name: pre-commit + +on: [pull_request] + +jobs: + pre-commit: + name: Pre-commit + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-python@v4 + with: + python-version: '3.10' + - run: python -m pip install pre-commit + - run: python -m pip freeze --local + - run: SKIP=clang-format pre-commit run --all-files diff --git a/.gitignore b/.gitignore index d46eda73..21269fe9 100644 --- a/.gitignore +++ b/.gitignore @@ -2,8 +2,8 @@ bin debug build # python environment used for testing -env -# cache files +env +# cache files *__pycache__* #ide *.code-workspace diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..024d60f8 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,22 @@ +# To install hooks, run: +# pre-commit install --hook-type pre-commit +# pre-commit install --hook-type commit-msg + +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.3.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-yaml + - id: check-added-large-files + - repo: https://github.com/pre-commit/mirrors-clang-format + rev: v17.0.4 + hooks: + - id: clang-format + name: clang-format + description: Run `clang-format` on C/C++/CUDA files. + entry: clang-format -i + language: system + files: ^ + types: [file, c] diff --git a/.vscode/settings.json b/.vscode/settings.json index f3844bfa..0d261338 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -7,4 +7,4 @@ "string": "c" }, "editor.formatOnSave": true -} \ No newline at end of file +} diff --git a/APDUs.md b/APDUs.md index cae5774b..a2d07d34 100644 --- a/APDUs.md +++ b/APDUs.md @@ -135,4 +135,3 @@ There are some limitations for Michelson parsing that should be noted. - “contract-to-contract” requires that you use: - the default endpoint for your destination contract - the parameters must be of type unit - diff --git a/README.md b/README.md index 9a28a58a..b87e9725 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ Download the source code for application from github repository [App-tezos-bakin $ git clone https://github.com/LedgerHQ/app-tezos.git $ cd app-tezos ``` -Then run the following command to enter into docker container provided by Ledger. You will need to have docker cli installed. +Then run the following command to enter into docker container provided by Ledger. You will need to have docker cli installed. Use the docker container `ledger-app-dev-tools` provided by Ledger to build the app. ``` @@ -255,17 +255,17 @@ Wallet application on the same Ledger device should suffice. ### Setup Ledger with Tezos client To connect ledger with Tezos client, you need to download [Tezos](https://www.gitlab.com/tezos/tezos). -You need to have nix installed on your system. Build tezos with following commands: +You need to have nix installed on your system. Build tezos with following commands: ``` $ git clone https://gitlab.com/tezos/tezos.git $ cd tezos $ nix-shell -j auto -$ make +$ make ``` This will build the latest version of tezos repo. Now connect the ledger device to USB port of your computer and run following command: ``` -$ ./octez-client list connected ledgger +$ ./octez-client list connected ledgger ``` It will given output as follows: ``` @@ -286,11 +286,11 @@ $ ./octez-client import secret key ledger_username "ledger://masculine-pig-stupe ``` Here we have chosen the last key type bip25519. You can choose any one of the available keys. -You can verify that you have successfully setup ledger with following command: +You can verify that you have successfully setup ledger with following command: ``` $ ./octez-client list known addresses ``` -It will show output as follows: +It will show output as follows: ``` ledger_<...>: tz1N4GQ8gYgMdq6gUsja783KJButHUHn5K7z (ledger sk known) ``` @@ -298,11 +298,11 @@ You can use the address ledger_<...> for further commands to setup the baking op ### Setup Node and baker -It is recommended to practice baking on tezos testnet before you acutally start baking on mainnet with real money. You can get more information +It is recommended to practice baking on tezos testnet before you acutally start baking on mainnet with real money. You can get more information about baking on testnet at [Baking-setup-Tutorial](https://docs.tezos.com/tutorials/join-dal-baker). Here we only give information about changes you have to make in above tutorial to bake with Ledger instead of an auto generated key. -In the tutorial skip the command `octez-client gen keys my_baker` and instead use the ledger_<...> in place of my_baker. +In the tutorial skip the command `octez-client gen keys my_baker` and instead use the ledger_<...> in place of my_baker. Use the following command to store your address in environmental variable `MY_BAKER` ``` $ MY_BAKER="$(./octez-client show address ledger_<...> | head -n 1 | cut -d ' ' -f 2)" @@ -344,10 +344,10 @@ $ octez-client register key ledger_<...> as delegate ``` This command is intended to inform the blockchain itself of your intention to -bake with this key. +bake with this key. ### Stake tez to get baking rights -Currently baking app does not support signing transactions. You need to stake certain amount of tez to get baking rights. Install Tezos wallet app on the same ledger device and run following command. No setup is needed as we have already setup the address from which we are deducting the amount. +Currently baking app does not support signing transactions. You need to stake certain amount of tez to get baking rights. Install Tezos wallet app on the same ledger device and run following command. No setup is needed as we have already setup the address from which we are deducting the amount. ``` $ octez-client stake ledger_<...> ``` @@ -381,7 +381,7 @@ attempt at signing; this operation is designed to be used unsupervised. As menti ### Security during baking -The Tezos-Baking app needs to be kept open during baking and ledger is unlocked during that time. To prevent screen burn, the baking app goes into blank screen when it starts signing blocks/attestation as baker. But the app remains unlocked. One can not sign any transaction operation using baking app, therefore there is no need of any concern. But to exit the baking app, one needs to enter PIN. This restriction is in place to avoid misuse of physical ledger device when its kept unattended during baking process. +The Tezos-Baking app needs to be kept open during baking and ledger is unlocked during that time. To prevent screen burn, the baking app goes into blank screen when it starts signing blocks/attestation as baker. But the app remains unlocked. One can not sign any transaction operation using baking app, therefore there is no need of any concern. But to exit the baking app, one needs to enter PIN. This restriction is in place to avoid misuse of physical ledger device when its kept unattended during baking process. ### Reset High Watermark @@ -529,4 +529,3 @@ If you install a Ledger application, such as Tezos Wallet or Tezos Baking, outsi ## Feedback To give feedback and report an error, create an issue on github repository [Trillitech-App-Tezos](https://github.com/trillitech/ledger-app-tezos-baking). - diff --git a/misra.json b/misra.json new file mode 100644 index 00000000..15f2e229 --- /dev/null +++ b/misra.json @@ -0,0 +1,7 @@ +{ + "script": "misra.py", + "args": [ + "--rule-texts=misra.md", + "--suppress-rules 2.7,3.1,7.2,7.4,8.2,8.4,8.9,8.14,9.2,10.1,10.3,10.4,10.5,10.6,11.1,11.3,11.5,11.8,12.1,12.3,13.3,13.4,14.2,14.4,15.5,16.3,16.4,16.5,16.6,17.7,17.8,18.4,18.8,19.2,20.5,20.7,21.15,21.16" + ] +} diff --git a/misra.md b/misra.md new file mode 100644 index 00000000..a3041a5c --- /dev/null +++ b/misra.md @@ -0,0 +1,728 @@ +# Appendix A Summary of guidelines + + +## The implementation + + +Dir 1.1 Required +Any implementation-defined behaviour on which the output of the +program depends shall be documented and understood + + +## Compilation and build + + +Dir 2.1 Required +All source files shall compile without any compilation errors + + +## Requirements traceability + + +Dir 3.1 Required +All code shall be traceable to documented requirements + + +## Code design + + +Dir 4.1 Required +Run-time failures shall be minimized + +Dir 4.2 Advisory +All usage of assembly language should be documented + +Dir 4.3 Required +Assembly language shall be encapsulated and isolated + +Dir 4.4 Advisory +Sections of code should not be “commented out” + +Dir 4.5 Advisory +Identifiers in the same name space with overlapping visibility should be +typographically unambiguous + +Dir 4.6 Advisory +typedefs that indicate size and signedness should be used in place of the +basic numerical types + +Dir 4.7 Required +If a function returns error information, then that error information shall +be tested + +Dir 4.8 Advisory +If a pointer to a structure or union is never dereferenced within a +translation unit, then the implementation of the object should be +hidden + +Dir 4.9 Advisory +A function should be used in preference to a function-like macro where +they are interchangeable + +Dir 4.10 Required +Precautions shall be taken in order to prevent the contents of a header +file being included more than once + +Dir 4.11 Required +The validity of values passed to library functions shall be checked + +Dir 4.12 Required +Dynamic memory allocation shall not be used + +Dir 4.13 Advisory +Functions which are designed to provide operations on a resource +should be called in an appropriate sequence + + +## A standard C environment + + +Rule 1.1 Required +The program shall contain no violations of the standard C syntax and +constraints, and shall not exceed the implementation’s translation limits + +Rule 1.2 Advisory +Language extensions should not be used + +Rule 1.3 Required +There shall be no occurrence of undefined or critical unspecified +behaviour + +Rule 1.4 Required +Emergent language features shall not be used + + +## Unused code + + +Rule 2.1 Required +A project shall not contain unreachable code + +Rule 2.2 Required +There shall be no dead code + +Rule 2.3 Advisory +A project should not contain unused type declarations + +Rule 2.4 Advisory +A project should not contain unused tag declarations + +Rule 2.5 Advisory +A project should not contain unused macro declarations + +Rule 2.6 Advisory +A function should not contain unused label declarations + +Rule 2.7 Advisory +There should be no unused parameters in functions + + +## Comments + + +Rule 3.1 Required +The character sequences /* and // shall not be used within a comment + +Rule 3.2 Required +Line-splicing shall not be used in // comments + + +## Character sets and lexical conventions + + +Rule 4.1 Required +Octal and hexadecimal escape sequences shall be terminated + +Rule 4.2 Advisory +Trigraphs should not be used + + +## Identifiers + + +Rule 5.1 Required +External identifiers shall be distinct + +Rule 5.2 Required +Identifiers declared in the same scope and name space shall be distinct + +Rule 5.3 Required +An identifier declared in an inner scope shall not hide an identifier +declared in an outer scope + +Rule 5.4 Required +Macro identifiers shall be distinct + +Rule 5.5 Required +Identifiers shall be distinct from macro names + +Rule 5.6 Required +A typedef name shall be a unique identifier + +Rule 5.7 Required +A tag name shall be a unique identifier + +Rule 5.8 Required +Identifiers that define objects or functions with external linkage shall be +unique + +Rule 5.9 Advisory +Identifiers that define objects or functions with internal linkage should +be unique + + +## Types + + +Rule 6.1 Required +Bit-fields shall only be declared with an appropriate type + +Rule 6.2 Required +Single-bit named bit fields shall not be of a signed type + + +## Literals and constants + + +Rule 7.1 Required +Octal constants shall not be used + +Rule 7.2 Required +A “u” or “U” suffix shall be applied to all integer constants that are +represented in an unsigned type + +Rule 7.3 Required +The lowercase character “l” shall not be used in a literal suffix + +Rule 7.4 Required +A string literal shall not be assigned to an object unless the object’s type +is “pointer to const-qualified char” + + +## Declarations and definitions + + +Rule 8.1 Required +Types shall be explicitly specified + +Rule 8.2 Required +Function types shall be in prototype form with named parameters + +Rule 8.3 Required +All declarations of an object or function shall use the same names and +type qualifiers + +Rule 8.4 Required +A compatible declaration shall be visible when an object or function +with external linkage is defined + +Rule 8.5 Required +An external object or function shall be declared once in one and only +one file + +Rule 8.6 Required +An identifier with external linkage shall have exactly one external +definition + +Rule 8.7 Advisory +Functions and objects should not be defined with external linkage if +they are referenced in only one translation unit + +Rule 8.8 Required +The static storage class specifier shall be used in all declarations of +objects and functions that have internal linkage + +Rule 8.9 Advisory +An object should be defined at block scope if its identifier only appears +in a single function + +Rule 8.10 Required +An inline function shall be declared with the static storage class + +Rule 8.11 Advisory +When an array with external linkage is declared, its size should be +explicitly specified + +Rule 8.12 Required +Within an enumerator list, the value of an implicitly-specified +enumeration constant shall be unique + +Rule 8.13 Advisory +A pointer should point to a const-qualified type whenever possible + +Rule 8.14 Required +The restrict type qualifier shall not be used + + +## Initialization + + +Rule 9.1 Mandatory +The value of an object with automatic storage duration shall not be read +before it has been set + +Rule 9.2 Required +The initializer for an aggregate or union shall be enclosed in braces + +Rule 9.3 Required +Arrays shall not be partially initialized + +Rule 9.4 Required +An element of an object shall not be initialized more than once + +Rule 9.5 Required +Where designated initializers are used to initialize an array object the +size of the array shall be specified explicitly + + +## The essential type model + + +Rule 10.1 Required +Operands shall not be of an inappropriate essential type + +Rule 10.2 Required +Expressions of essentially character type shall not be used +inappropriately in addition and subtraction operations + +Rule 10.3 Required +The value of an expression shall not be assigned to an object with a +narrower essential type or of a different essential type category + +Rule 10.4 Required +Both operands of an operator in which the usual arithmetic conversions +are performed shall have the same essential type category + +Rule 10.5 Advisory +The value of an expression should not be cast to an inappropriate +essential type + +Rule 10.6 Required +The value of a composite expression shall not be assigned to an object +with wider essential type + +Rule 10.7 Required +If a composite expression is used as one operand of an operator in which +the usual arithmetic conversions are performed then the other operand +shall not have wider essential type + +Rule 10.8 Required +The value of a composite expression shall not be cast to a different +essential type category or a wider essential type + + +## Pointer type conversions + + +Rule 11.1 Required +Conversions shall not be performed between a pointer to a function +and any other type + +Rule 11.2 Required +Conversions shall not be performed between a pointer to an +incomplete type and any other type + +Rule 11.3 Required +A cast shall not be performed between a pointer to object type and a +pointer to a diff erent object type + +Rule 11.4 Advisory +A conversion should not be performed between a pointer to object and +an integer type + +Rule 11.5 Advisory +A conversion should not be performed from pointer to void into pointer +to object + +Rule 11.6 Required +A cast shall not be performed between pointer to void and an arithmetic +type + +Rule 11.7 Required +A cast shall not be performed between pointer to object and a non- +integer arithmetic type + +Rule 11.8 Required +A cast shall not remove any const or volatile qualification from the type +pointed to by a pointer + +Rule 11.9 Required +The macro NULL shall be the only permitted form of integer null pointer +constant + + +## Expressions + + +Rule 12.1 Advisory +The precedence of operators within expressions should be made +explicit + +Rule 12.2 Required +The right hand operand of a shift operator shall lie in the range zero +to one less than the width in bits of the essential type of the left hand +operand + +Rule 12.3 Advisory +The comma operator should not be used + +Rule 12.4 Advisory +Evaluation of constant expressions should not lead to unsigned integer +wrap-around + + +## Side effects + + +Rule 13.1 Required +Initializer lists shall not contain persistent side effects + +Rule 13.2 Required +The value of an expression and its persistent side effects shall be the +same under all permitted evaluation orders + +Rule 13.3 Advisory +A full expression containing an increment (++) or decrement (--) +operator should have no other potential side effects other than that +caused by the increment or decrement operator + +Rule 13.4 Advisory +The result of an assignment operator should not be used + +Rule 13.5 Required +The right hand operand of a logical && or || operator shall not contain +persistent side effects + +Rule 13.6 Mandatory +The operand of the sizeof operator shall not contain any expression +which has potential side effects + + +## Control statement expressions + + +Rule 14.1 Required +A loop counter shall not have essentially floating type + +Rule 14.2 Required +A for loop shall be well-formed + +Rule 14.3 Required +Controlling expressions shall not be invariant + +Rule 14.4 Required +The controlling expression of an if statement and the controlling +expression of an iteration-statement shall have essentially Boolean type + + +## Control flow + + +Rule 15.1 Advisory +The goto statement should not be used + +Rule 15.2 Required +The goto statement shall jump to a label declared later in the same +function + +Rule 15.3 Required +Any label referenced by a goto statement shall be declared in the same +block, or in any block enclosing the goto statement + +Rule 15.4 Advisory +There should be no more than one break or goto statement used to +terminate any iteration statement + +Rule 15.5 Advisory +A function should have a single point of exit at the end + +Rule 15.6 Required +The body of an iteration-statement or a selection-statement shall be a +compound-statement + +Rule 15.7 Required +All if … else if constructs shall be terminated with an else statement + + +## Switch statements + + +Rule 16.1 Required +All switch statements shall be well-formed + +Rule 16.2 Required +A switch label shall only be used when the most closely-enclosing +compound statement is the body of a switch statement + +Rule 16.3 Required +An unconditional break statement shall terminate every switch-clause + +Rule 16.4 Required +Every switch statement shall have a default label + +Rule 16.5 Required +A default label shall appear as either the first or the last switch label of a +switch statement + +Rule 16.6 Required +Every switch statement shall have at least two switch-clauses + +Rule 16.7 Required +A switch-expression shall not have essentially Boolean type + + +## Functions + + +Rule 17.1 Required +The features of shall not be used + +Rule 17.2 Required +Functions shall not call themselves, either directly or indirectly + +Rule 17.3 Mandatory +A function shall not be declared implicitly + +Rule 17.4 Mandatory +All exit paths from a function with non-void return type shall have an +explicit return statement with an expression + +Rule 17.5 Advisory +The function argument corresponding to a parameter declared to have +an array type shall have an appropriate number of elements + +Rule 17.6 Mandatory +The declaration of an array parameter shall not contain the static +keyword between the [ ] + +Rule 17.7 Required +The value returned by a function having non-void return type shall be +used + +Rule 17.8 Advisory +A function parameter should not be modified + + +## Pointers and arrays + + +Rule 18.1 Required +A pointer resulting from arithmetic on a pointer operand shall address +an element of the same array as that pointer operand + +Rule 18.2 Required +Subtraction between pointers shall only be applied to pointers that +address elements of the same array + +Rule 18.3 Required +The relational operators >, >=, < and <= shall not be applied to objects +of pointer type except where they point into the same object + +Rule 18.4 Advisory +The +, -, += and -= operators should not be applied to an expression of +pointer type + +Rule 18.5 Advisory +Declarations should contain no more than two levels of pointer nesting + +Rule 18.6 Required +The address of an object with automatic storage shall not be copied to +another object that persists after the first object has ceased to exist + +Rule 18.7 Required +Flexible array members shall not be declared + +Rule 18.8 Required +Variable-length array types shall not be used + + +## Overlapping storage + + +Rule 19.1 Mandatory +An object shall not be assigned or copied to an overlapping object + +Rule 19.2 Advisory +The union keyword should not be used + + +## Preprocessing directives + + +Rule 20.1 Advisory +#include directives should only be preceded by preprocessor directives +or comments + +Rule 20.2 Required +The ', " or \ characters and the /* or // character sequences shall not +occur in a header file name + +Rule 20.3 Required +The #include directive shall be followed by either a or +"filename" sequence + +Rule 20.4 Required +A macro shall not be defined with the same name as a keyword + +Rule 20.5 Advisory +#undef should not be used + +Rule 20.6 Required +Tokens that look like a preprocessing directive shall not occur within a +macro argument + +Rule 20.7 Required +Expressions resulting from the expansion of macro parameters shall be +enclosed in parentheses + +Rule 20.8 Required +The controlling expression of a #if or #elif preprocessing directive shall +evaluate to 0 or 1 + +Rule 20.9 Required +All identifiers used in the controlling expression of #if or #elif +preprocessing directives shall be #define’d before evaluation + +Rule 20.10 Advisory +The # and ## preprocessor operators should not be used + +Rule 20.11 Required +A macro parameter immediately following a # operator shall not +immediately be followed by a ## operator + +Rule 20.12 Required +A macro parameter used as an operand to the # or ## operators, which +is itself subject to further macro replacement, shall only be used as an +operand to these operators + +Rule 20.13 Required +A line whose first token is # shall be a valid preprocessing directive + +Rule 20.14 Required +All #else, #elif and #endif preprocessor directives shall reside in the +same file as the #if, #ifdef or #ifndef directive to which they are related + + +## Standard libraries + + +Rule 21.1 Required +#define and #undef shall not be used on a reserved identifier or +reserved macro name + +Rule 21.2 Required +A reserved identifier or macro name shall not be declared + +Rule 21.3 Required +The memory allocation and deallocation functions of shall +not be used + +Rule 21.4 Required +The standard header file shall not be used + +Rule 21.5 Required +The standard header file shall not be used + +Rule 21.6 Required +The Standard Library input/output functions shall not be used + +Rule 21.7 Required +The atof, atoi, atol and atoll functions of shall not be used + +Rule 21.8 Required +The Standard Library termination functions of shall not +be used + +Rule 21.9 Required +The library functions bsearch and qsort of shall not be +used + +Rule 21.10 Required +The Standard Library time and date functions shall not be used + +Rule 21.11 Required +The standard header file shall not be used + +Rule 21.12 Advisory +The exception handling features of should not be used + +Rule 21.13 Mandatory +Any value passed to a function in shall be representable as +an unsigned char or be the value EOF + +Rule 21.14 Required +The Standard Library function memcmp shall not be used to compare null +terminated strings + +Rule 21.15 Required +The pointer arguments to the Standard Library functions memcpy, +memmove and memcmp shall be pointers to qualified or unqualified +versions of compatible types + +Rule 21.16 Required +The pointer arguments to the Standard Library function memcmp shall +point to either a pointer type, an essentially signed type, an +essentially unsigned type, an essentially Boolean type or an +essentially enum type + +Rule 21.17 Mandatory +Use of the string handling functions from shall not result +in accesses beyond the bounds of the objects referenced by their +pointer parameters + +Rule 21.18 Mandatory +The size_t argument passed to any function in shall have an +appropriate value + +Rule 21.19 Mandatory +The pointers returned by the Standard Library functions localeconv, +getenv, setlocale or, strerror shall only be used as if they have +pointer to const-qualified type + +Rule 21.20 Mandatory +The pointer returned by the Standard Library functions asctime, ctime, +gmtime, localtime, localeconv, getenv, setlocale or strerror shall not +be used following a subsequent call to the same function + +Rule 21.21 Required +The Standard Library function system of shall not be +used + + +## Resources + + +Rule 22.1 Required +All resources obtained dynamically by means of Standard Library +functions shall be explicitly released + +Rule 22.2 Mandatory +A block of memory shall only be freed if it was allocated by means of a +Standard Library function + +Rule 22.3 Required +The same file shall not be open for read and write access at the same +time on different streams + +Rule 22.4 Mandatory +There shall be no attempt to write to a stream which has been opened +as read-only + +Rule 22.5 Mandatory +A pointer to a FILE object shall not be dereferenced + +Rule 22.6 Mandatory +The value of a pointer to a FILE shall not be used after the associated +stream has been closed + +Rule 22.7 Required +The macro EOF shall only be compared with the unmodified return value from any Standard Library function capable of returning EOF + +Rule 22.8 Required +The value of errno shall be set to zero prior to a call to an errno-setting-function + +Rule 22.9 Required +The value of errno shall be tested against zero after calling an errno-setting-function + +Rule 22.10 Required +The value of errno shall only be tested when the last function to be called was an errno-setting-function diff --git a/src/apdu_baking.c b/src/apdu_baking.c index 112532ed..1f3b9e92 100644 --- a/src/apdu_baking.c +++ b/src/apdu_baking.c @@ -19,7 +19,9 @@ size_t handle_apdu_reset(__attribute__((unused)) uint8_t instruction, volatile u THROW(EXC_WRONG_LENGTH_FOR_INS); } level_t const lvl = READ_UNALIGNED_BIG_ENDIAN(level_t, dataBuffer); - if (!is_valid_level(lvl)) THROW(EXC_PARSE_ERROR); + if (!is_valid_level(lvl)) { + THROW(EXC_PARSE_ERROR); + } G.reset_level = lvl; ui_baking_reset(flags); @@ -61,9 +63,13 @@ size_t handle_apdu_all_hwm(__attribute__((unused)) uint8_t instruction, tx = send_word_big_endian(tx, N_data.hwm.main.highest_level); int has_a_chain_migrated = N_data.hwm.main.migrated_to_tenderbake || N_data.hwm.test.migrated_to_tenderbake; - if (has_a_chain_migrated) tx = send_word_big_endian(tx, N_data.hwm.main.highest_round); + if (has_a_chain_migrated) { + tx = send_word_big_endian(tx, N_data.hwm.main.highest_round); + } tx = send_word_big_endian(tx, N_data.hwm.test.highest_level); - if (has_a_chain_migrated) tx = send_word_big_endian(tx, N_data.hwm.test.highest_round); + if (has_a_chain_migrated) { + tx = send_word_big_endian(tx, N_data.hwm.test.highest_round); + } tx = send_word_big_endian(tx, N_data.main_chain_id.v); return finalize_successful_send(tx); } @@ -72,8 +78,9 @@ size_t handle_apdu_main_hwm(__attribute__((unused)) uint8_t instruction, __attribute__((unused)) volatile uint32_t* flags) { size_t tx = 0; tx = send_word_big_endian(tx, N_data.hwm.main.highest_level); - if (N_data.hwm.main.migrated_to_tenderbake) + if (N_data.hwm.main.migrated_to_tenderbake) { tx = send_word_big_endian(tx, N_data.hwm.main.highest_round); + } return finalize_successful_send(tx); } @@ -107,8 +114,12 @@ size_t handle_apdu_query_auth_key_with_curve(__attribute__((unused)) uint8_t ins size_t handle_apdu_deauthorize(__attribute__((unused)) uint8_t instruction, __attribute__((unused)) volatile uint32_t* flags) { - if (G_io_apdu_buffer[OFFSET_P1] != 0) THROW(EXC_WRONG_PARAM); - if (G_io_apdu_buffer[OFFSET_LC] != 0) THROW(EXC_PARSE_ERROR); + if (G_io_apdu_buffer[OFFSET_P1] != 0) { + THROW(EXC_WRONG_PARAM); + } + if (G_io_apdu_buffer[OFFSET_LC] != 0) { + THROW(EXC_PARSE_ERROR); + } UPDATE_NVRAM(ram, { memset(&ram->baking_key, 0, sizeof(ram->baking_key)); }); return finalize_successful_send(0); diff --git a/src/apdu_hmac.c b/src/apdu_hmac.c index c22427f1..ea82b5c6 100644 --- a/src/apdu_hmac.c +++ b/src/apdu_hmac.c @@ -15,7 +15,9 @@ static inline size_t hmac(uint8_t *const out, check_null(out); check_null(state); check_null(in); - if (out_size < CX_SHA256_SIZE) THROW(EXC_WRONG_LENGTH); + if (out_size < CX_SHA256_SIZE) { + THROW(EXC_WRONG_LENGTH); + } // Pick a static, arbitrary SHA256 value based on a quote of Jesus. static uint8_t const key_sha256[] = {0x6c, 0x4e, 0x7e, 0x70, 0x6c, 0x54, 0xd3, 0x67, @@ -63,11 +65,15 @@ static inline size_t hmac(uint8_t *const out, size_t handle_apdu_hmac(__attribute__((unused)) uint8_t instruction, __attribute__((unused)) volatile uint32_t *flags) { - if (G_io_apdu_buffer[OFFSET_P1] != 0) THROW(EXC_WRONG_PARAM); + if (G_io_apdu_buffer[OFFSET_P1] != 0) { + THROW(EXC_WRONG_PARAM); + } uint8_t const *const buff = &G_io_apdu_buffer[OFFSET_CDATA]; uint8_t const buff_size = G_io_apdu_buffer[OFFSET_LC]; - if (buff_size > MAX_APDU_SIZE) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (buff_size > MAX_APDU_SIZE) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } memset(&G, 0, sizeof(G)); diff --git a/src/apdu_pubkey.c b/src/apdu_pubkey.c index 4507564e..a3232c89 100644 --- a/src/apdu_pubkey.c +++ b/src/apdu_pubkey.c @@ -38,10 +38,14 @@ char const *const *get_baking_prompts() { size_t handle_apdu_get_public_key(uint8_t instruction, volatile uint32_t *flags) { uint8_t *dataBuffer = G_io_apdu_buffer + OFFSET_CDATA; - if (G_io_apdu_buffer[OFFSET_P1] != 0) THROW(EXC_WRONG_PARAM); + if (G_io_apdu_buffer[OFFSET_P1] != 0) { + THROW(EXC_WRONG_PARAM); + } // do not expose pks without prompt through U2F (permissionless legacy comm in browser) - if (instruction == INS_GET_PUBLIC_KEY) require_permissioned_comm(); + if (instruction == INS_GET_PUBLIC_KEY) { + require_permissioned_comm(); + } global.path_with_curve.derivation_type = parse_derivation_type(G_io_apdu_buffer[OFFSET_CURVE]); @@ -51,7 +55,9 @@ size_t handle_apdu_get_public_key(uint8_t instruction, volatile uint32_t *flags) copy_bip32_path_with_curve(&global.path_with_curve, &N_data.baking_key); } else { read_bip32_path(&global.path_with_curve.bip32_path, dataBuffer, cdata_size); - if (global.path_with_curve.bip32_path.length == 0) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (global.path_with_curve.bip32_path.length == 0) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } } cx_ecfp_public_key_t public_key = {0}; diff --git a/src/apdu_setup.c b/src/apdu_setup.c index 0d1cdeee..c6605e39 100644 --- a/src/apdu_setup.c +++ b/src/apdu_setup.c @@ -45,10 +45,14 @@ static bool ok(void) { } size_t handle_apdu_setup(__attribute__((unused)) uint8_t instruction, volatile uint32_t *flags) { - if (G_io_apdu_buffer[OFFSET_P1] != 0) THROW(EXC_WRONG_PARAM); + if (G_io_apdu_buffer[OFFSET_P1] != 0) { + THROW(EXC_WRONG_PARAM); + } uint32_t const buff_size = G_io_apdu_buffer[OFFSET_LC]; - if (buff_size < sizeof(struct setup_wire)) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (buff_size < sizeof(struct setup_wire)) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } global.path_with_curve.derivation_type = parse_derivation_type(G_io_apdu_buffer[OFFSET_CURVE]); @@ -71,7 +75,9 @@ size_t handle_apdu_setup(__attribute__((unused)) uint8_t instruction, volatile u (uint8_t const *) &buff_as_setup->bip32_path, buff_size - consumed); - if (consumed != buff_size) THROW(EXC_WRONG_LENGTH); + if (consumed != buff_size) { + THROW(EXC_WRONG_LENGTH); + } } prompt_setup(ok, delay_reject); diff --git a/src/apdu_sign.c b/src/apdu_sign.c index e686ae3f..323d3d1a 100644 --- a/src/apdu_sign.c +++ b/src/apdu_sign.c @@ -40,7 +40,9 @@ static void blake2b_incremental_hash( uint8_t *current = out; while (*out_length > B2B_BLOCKBYTES) { - if (current - out > (int) out_size) THROW(EXC_MEMORY_ERROR); + if (current - out > (int) out_size) { + THROW(EXC_MEMORY_ERROR); + } conditional_init_hash_state(state); CX_THROW( cx_hash_no_throw((cx_hash_t *) &state->state, 0, current, B2B_BLOCKBYTES, NULL, 0)); @@ -101,7 +103,9 @@ size_t baking_sign_complete(bool const send_hash, volatile uint32_t *flags) { break; case MAGIC_BYTE_UNSAFE_OP: { - if (!G.maybe_ops.is_valid) PARSE_ERROR(); + if (!G.maybe_ops.is_valid) { + PARSE_ERROR(); + } switch (G.maybe_ops.v.operation.tag) { case OPERATION_TAG_DELEGATION: @@ -171,7 +175,9 @@ static size_t handle_apdu(bool const enable_hashing, uint8_t *const buff = &G_io_apdu_buffer[OFFSET_CDATA]; uint8_t const p1 = G_io_apdu_buffer[OFFSET_P1]; uint8_t const buff_size = G_io_apdu_buffer[OFFSET_LC]; - if (buff_size > MAX_APDU_SIZE) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (buff_size > MAX_APDU_SIZE) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } bool last = (p1 & P1_LAST_MARKER) != 0; switch (p1 & ~P1_LAST_MARKER) { @@ -182,10 +188,14 @@ static size_t handle_apdu(bool const enable_hashing, parse_derivation_type(G_io_apdu_buffer[OFFSET_CURVE]); return finalize_successful_send(0); case P1_NEXT: - if (global.path_with_curve.bip32_path.length == 0) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (global.path_with_curve.bip32_path.length == 0) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } // Guard against overflow - if (G.packet_index >= 0xFF) PARSE_ERROR(); + if (G.packet_index >= 0xFF) { + PARSE_ERROR(); + } G.packet_index++; break; @@ -194,7 +204,9 @@ static size_t handle_apdu(bool const enable_hashing, } if (enable_parsing) { - if (G.packet_index != 1) PARSE_ERROR(); // Only parse a single packet when baking + if (G.packet_index != 1) { + PARSE_ERROR(); // Only parse a single packet when baking + } G.magic_byte = get_magic_byte_or_throw(buff, buff_size); if (G.magic_byte == MAGIC_BYTE_UNSAFE_OP) { @@ -206,7 +218,9 @@ static size_t handle_apdu(bool const enable_hashing, &global.path_with_curve.bip32_path); } else { // This should be a baking operation so parse it. - if (!parse_baking_data(&G.parsed_baking_data, buff, buff_size)) PARSE_ERROR(); + if (!parse_baking_data(&G.parsed_baking_data, buff, buff_size)) { + PARSE_ERROR(); + } } } @@ -218,7 +232,9 @@ static size_t handle_apdu(bool const enable_hashing, &G.hash_state); } - if (G.message_data_length + buff_size > sizeof(G.message_data)) PARSE_ERROR(); + if (G.message_data_length + buff_size > sizeof(G.message_data)) { + PARSE_ERROR(); + } memmove(G.message_data + G.message_data_length, buff, buff_size); G.message_data_length += buff_size; diff --git a/src/baking_auth.c b/src/baking_auth.c index 5c6f6a68..9c4333de 100644 --- a/src/baking_auth.c +++ b/src/baking_auth.c @@ -18,7 +18,9 @@ bool is_valid_level(level_t lvl) { void write_high_water_mark(parsed_baking_data_t const *const in) { check_null(in); - if (!is_valid_level(in->level)) THROW(EXC_WRONG_VALUES); + if (!is_valid_level(in->level)) { + THROW(EXC_WRONG_VALUES); + } UPDATE_NVRAM(ram, { // If the chain matches the main chain *or* the main chain is not set, then use 'main' HWM. high_watermark_t volatile *const dest = select_hwm_by_chain(in->chain_id, ram); @@ -38,8 +40,9 @@ void authorize_baking(derivation_type_t const derivation_type, bip32_path_t const *const bip32_path) { check_null(bip32_path); if (bip32_path->length > NUM_ELEMENTS(N_data.baking_key.bip32_path.components) || - bip32_path->length == 0) + bip32_path->length == 0) { return; + } UPDATE_NVRAM(ram, { ram->baking_key.derivation_type = derivation_type; @@ -49,7 +52,9 @@ void authorize_baking(derivation_type_t const derivation_type, static bool is_level_authorized(parsed_baking_data_t const *const baking_info) { check_null(baking_info); - if (!is_valid_level(baking_info->level)) return false; + if (!is_valid_level(baking_info->level)) { + return false; + } high_watermark_t volatile const *const hwm = select_hwm_by_chain(baking_info->chain_id, &N_data); @@ -89,8 +94,12 @@ void guard_baking_authorized(parsed_baking_data_t const *const baking_info, bip32_path_with_curve_t const *const key) { check_null(baking_info); check_null(key); - if (!is_path_authorized(key->derivation_type, &key->bip32_path)) THROW(EXC_SECURITY); - if (!is_level_authorized(baking_info)) THROW(EXC_WRONG_VALUES); + if (!is_path_authorized(key->derivation_type, &key->bip32_path)) { + THROW(EXC_SECURITY); + } + if (!is_level_authorized(baking_info)) { + THROW(EXC_WRONG_VALUES); + } } struct block_wire { @@ -141,7 +150,9 @@ uint8_t get_proto_version(void const *const fitness) { } bool parse_block(parsed_baking_data_t *const out, void const *const data, size_t const length) { - if (length < sizeof(struct block_wire) + MINIMUM_FITNESS_SIZE) return false; + if (length < sizeof(struct block_wire) + MINIMUM_FITNESS_SIZE) { + return false; + } struct block_wire const *const block = data; void const *const fitness = data + sizeof(struct block_wire); uint8_t proto_version = get_proto_version(fitness); @@ -166,7 +177,9 @@ bool parse_block(parsed_baking_data_t *const out, void const *const data, size_t bool parse_consensus_operation(parsed_baking_data_t *const out, void const *const data, size_t const length) { - if (length < sizeof(struct consensus_op_wire)) return false; + if (length < sizeof(struct consensus_op_wire)) { + return false; + } struct consensus_op_wire const *const op = data; out->chain_id.v = READ_UNALIGNED_BIG_ENDIAN(uint32_t, &op->chain_id); diff --git a/src/base58_encoding.c b/src/base58_encoding.c index 72028f3c..939aab24 100644 --- a/src/base58_encoding.c +++ b/src/base58_encoding.c @@ -25,7 +25,9 @@ bool b58enc(/* out */ char *b58, /* in/out */ size_t *b58sz, const void *data, s size_t size; uint8_t buf[MAX_ENC_INPUT_SIZE * 138 / 100 + 1] = {0}; - while (zcount < binsz && !bin[zcount]) ++zcount; + while (zcount < binsz && !bin[zcount]) { + ++zcount; + } size = (binsz - zcount) * 138 / 100 + 1; @@ -41,16 +43,21 @@ bool b58enc(/* out */ char *b58, /* in/out */ size_t *b58sz, const void *data, s } } - for (j = 0; j < size && !buf[j]; ++j) - ; + for (j = 0; (j < size) && !buf[j]; ++j) { + // Find the last index of buf regarding the size + } if (*b58sz <= zcount + size - j) { *b58sz = zcount + size - j + 1; return false; } - if (zcount) memset(b58, '1', zcount); - for (i = zcount; j < size; ++i, ++j) b58[i] = b58digits_ordered[buf[j]]; + if (zcount) { + memset(b58, '1', zcount); + } + for (i = zcount; j < size; ++i, ++j) { + b58[i] = b58digits_ordered[buf[j]]; + } b58[i] = '\0'; *b58sz = i + 1; diff --git a/src/globals.c b/src/globals.c index 317ee2da..116c7776 100644 --- a/src/globals.c +++ b/src/globals.c @@ -64,7 +64,7 @@ void copy_key(char *out, size_t out_size, void *data) { cx_ecfp_public_key_t pubkey = {0}; generate_public_key(&pubkey, (derivation_type_t const) baking_key->derivation_type, - (bip32_path_t const *const) & baking_key->bip32_path); + (bip32_path_t const *const) &baking_key->bip32_path); pubkey_to_pkh_string(out, out_size, (derivation_type_t const) baking_key->derivation_type, diff --git a/src/globals.h b/src/globals.h index d2e5b43d..ad52a020 100644 --- a/src/globals.h +++ b/src/globals.h @@ -148,7 +148,7 @@ high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id, ({ \ nvram_data *const out_name = &global.apdu.baking_auth.new_data; \ memcpy(&global.apdu.baking_auth.new_data, \ - (nvram_data const *const) & N_data, \ + (nvram_data const *const) &N_data, \ sizeof(global.apdu.baking_auth.new_data)); \ body; \ nvm_write((void *) &N_data, &global.apdu.baking_auth.new_data, sizeof(N_data)); \ diff --git a/src/keys.c b/src/keys.c index 61c5107b..17de95c4 100644 --- a/src/keys.c +++ b/src/keys.c @@ -29,14 +29,19 @@ size_t read_bip32_path(bip32_path_t *const out, uint8_t const *const in, size_t const in_size) { struct bip32_path_wire const *const buf_as_bip32 = (struct bip32_path_wire const *) in; - if (in_size < sizeof(buf_as_bip32->length)) THROW(EXC_WRONG_LENGTH_FOR_INS); + if (in_size < sizeof(buf_as_bip32->length)) { + THROW(EXC_WRONG_LENGTH_FOR_INS); + } size_t ix = 0; out->length = CONSUME_UNALIGNED_BIG_ENDIAN(ix, uint8_t, &buf_as_bip32->length); - if (in_size - ix < out->length * sizeof(*buf_as_bip32->components)) + if (in_size - ix < out->length * sizeof(*buf_as_bip32->components)) { THROW(EXC_WRONG_LENGTH_FOR_INS); - if (out->length == 0 || out->length > NUM_ELEMENTS(out->components)) THROW(EXC_WRONG_VALUES); + } + if (out->length == 0 || out->length > NUM_ELEMENTS(out->components)) { + THROW(EXC_WRONG_VALUES); + } for (size_t j = 0; j < out->length; j++) { out->components[j] = @@ -75,9 +80,10 @@ int crypto_derive_private_key(cx_ecfp_private_key_t *private_key, NULL); } - if (!error) + if (!error) { // new private_key from raw error = cx_ecfp_init_private_key_no_throw(cx_curve, raw_private_key, 32, private_key); + } explicit_bzero(raw_private_key, sizeof(raw_private_key)); @@ -144,7 +150,9 @@ void public_key_hash(uint8_t *const hash_out, cx_ecfp_public_key_t const *const restrict public_key) { check_null(hash_out); check_null(public_key); - if (hash_out_size < HASH_SIZE) THROW(EXC_WRONG_LENGTH); + if (hash_out_size < HASH_SIZE) { + THROW(EXC_WRONG_LENGTH); + } cx_ecfp_public_key_t compressed = {0}; switch (derivation_type_to_signature_type(derivation_type)) { @@ -192,7 +200,9 @@ size_t sign(uint8_t *const out, switch (derivation_type_to_signature_type(derivation_type)) { case SIGNATURE_TYPE_ED25519: { static size_t const SIG_SIZE = 64; - if (out_size < SIG_SIZE) THROW(EXC_WRONG_LENGTH); + if (out_size < SIG_SIZE) { + THROW(EXC_WRONG_LENGTH); + } CX_THROW(cx_eddsa_sign_no_throw(&pair->private_key, CX_SHA512, @@ -207,7 +217,9 @@ size_t sign(uint8_t *const out, case SIGNATURE_TYPE_SECP256K1: case SIGNATURE_TYPE_SECP256R1: { static size_t const SIG_SIZE = 100; - if (out_size < SIG_SIZE) THROW(EXC_WRONG_LENGTH); + if (out_size < SIG_SIZE) { + THROW(EXC_WRONG_LENGTH); + } unsigned int info; size_t sig_len = SIG_SIZE; CX_THROW(cx_ecdsa_sign_no_throw(&pair->private_key, diff --git a/src/keys.h b/src/keys.h index c330159c..7288a58f 100644 --- a/src/keys.h +++ b/src/keys.h @@ -99,4 +99,4 @@ static inline cx_curve_t signature_type_to_cx_curve(signature_type_t const signa int generate_public_key(cx_ecfp_public_key_t *public_key, derivation_type_t const derivation_type, - bip32_path_t const *const bip32_path); \ No newline at end of file + bip32_path_t const *const bip32_path); diff --git a/src/operations.c b/src/operations.c index b8b0d125..cd6132bf 100644 --- a/src/operations.c +++ b/src/operations.c @@ -51,7 +51,9 @@ static inline void compute_pkh(cx_ecfp_public_key_t *const compressed_pubkey_out derivation_type, &pubkey); contract_out->signature_type = derivation_type_to_signature_type(derivation_type); - if (contract_out->signature_type == SIGNATURE_TYPE_UNSET) THROW(EXC_MEMORY_ERROR); + if (contract_out->signature_type == SIGNATURE_TYPE_UNSET) { + THROW(EXC_MEMORY_ERROR); + } contract_out->originated = 0; } @@ -65,7 +67,9 @@ static inline void parse_implicit(parsed_contract_t *const out, } #define CALL_SUBPARSER_LN(func, line, ...) \ - if (func(__VA_ARGS__, line)) return true + if (func(__VA_ARGS__, line)) { \ + return true; \ + } #define CALL_SUBPARSER(func, ...) CALL_SUBPARSER_LN(func, __LINE__, __VA_ARGS__) // Subparsers: no function here should be called anywhere in this file without using the @@ -105,9 +109,10 @@ static inline bool parse_next_type(uint8_t current_byte, uint32_t sizeof_type, uint32_t lineno) { #ifdef DEBUG - if (sizeof_type > sizeof(state->body)) + if (sizeof_type > sizeof(state->body)) { PARSE_ERROR(); // Shouldn't happen, but error if it does and we're debugging. Neither side // is dynamic. + } #endif if (state->lineno != lineno) { @@ -207,7 +212,9 @@ static inline bool parse_byte(uint8_t byte, case 0: { // Verify magic byte, ignore block hash const struct operation_group_header *ogh = NEXT_TYPE(struct operation_group_header); - if (ogh->magic_byte != MAGIC_BYTE_UNSAFE_OP) PARSE_ERROR(); + if (ogh->magic_byte != MAGIC_BYTE_UNSAFE_OP) { + PARSE_ERROR(); + } } OP_NAMED_STEP(1) @@ -239,7 +246,9 @@ static inline bool parse_byte(uint8_t byte, // If the source is an implicit contract,... if (out->operation.source.originated == 0) { // ... it had better match our key, otherwise why are we signing it? - if (COMPARE(&out->operation.source, &out->signing) != 0) PARSE_ERROR(); + if (COMPARE(&out->operation.source, &out->signing) != 0) { + PARSE_ERROR(); + } } // OK, it passes muster. @@ -267,8 +276,10 @@ static inline bool parse_byte(uint8_t byte, { raw_tezos_header_signature_type_t const *const sig_type = NEXT_TYPE(raw_tezos_header_signature_type_t); - if (parse_raw_tezos_header_signature_type(sig_type) != out->signing.signature_type) + if (parse_raw_tezos_header_signature_type(sig_type) != + out->signing.signature_type) { PARSE_ERROR(); + } } OP_STEP @@ -279,8 +290,9 @@ static inline bool parse_byte(uint8_t byte, CALL_SUBPARSER(parse_next_type, byte, &(state->subparser_state.nexttype), klen); if (memcmp(out->public_key.W, &(state->subparser_state.nexttype.body.raw), klen) != - 0) + 0) { PARSE_ERROR(); + } out->has_reveal = true; @@ -353,7 +365,9 @@ static void parse_operations_throws_parse_error(struct parsed_operation_group *c ix++; } - if (!parse_operations_final(&G.parse_state, out)) PARSE_ERROR(); + if (!parse_operations_final(&G.parse_state, out)) { + PARSE_ERROR(); + } } bool parse_operations(struct parsed_operation_group *const out, diff --git a/src/to_string.c b/src/to_string.c index 09923247..447fed75 100644 --- a/src/to_string.c +++ b/src/to_string.c @@ -66,7 +66,9 @@ void pkh_to_string(char *const buff, uint8_t const hash[HASH_SIZE]) { check_null(buff); check_null(hash); - if (buff_size < PKH_STRING_SIZE) THROW(EXC_WRONG_LENGTH); + if (buff_size < PKH_STRING_SIZE) { + THROW(EXC_WRONG_LENGTH); + } // Data to encode struct __attribute__((packed)) { @@ -106,32 +108,40 @@ void pkh_to_string(char *const buff, compute_hash_checksum(data.checksum, &data, sizeof(data) - sizeof(data.checksum)); size_t out_size = buff_size; - if (!b58enc(buff, &out_size, &data, sizeof(data))) THROW(EXC_WRONG_LENGTH); + if (!b58enc(buff, &out_size, &data, sizeof(data))) { + THROW(EXC_WRONG_LENGTH); + } } void chain_id_to_string(char *const buff, size_t const buff_size, chain_id_t const chain_id) { check_null(buff); - if (buff_size < CHAIN_ID_BASE58_STRING_SIZE) THROW(EXC_WRONG_LENGTH); + if (buff_size < CHAIN_ID_BASE58_STRING_SIZE) { + THROW(EXC_WRONG_LENGTH); + } + + // Must hash big-endian data so treating little endian as big endian just flips + uint32_t chain_id_value = READ_UNALIGNED_BIG_ENDIAN(uint32_t, &chain_id.v); // Data to encode struct __attribute__((packed)) { uint8_t prefix[3]; int32_t chain_id; uint8_t checksum[TEZOS_HASH_CHECKSUM_SIZE]; - } data = {.prefix = {87, 82, 0}, - - // Must hash big-endian data so treating little endian as big endian just flips - .chain_id = READ_UNALIGNED_BIG_ENDIAN(uint32_t, &chain_id.v)}; + } data = {.prefix = {87, 82, 0}, .chain_id = chain_id_value}; compute_hash_checksum(data.checksum, &data, sizeof(data) - sizeof(data.checksum)); size_t out_size = buff_size; - if (!b58enc(buff, &out_size, &data, sizeof(data))) THROW(EXC_WRONG_LENGTH); + if (!b58enc(buff, &out_size, &data, sizeof(data))) { + THROW(EXC_WRONG_LENGTH); + } } #define STRCPY_OR_THROW(buff, size, x, exc) \ ({ \ - if (size < sizeof(x)) THROW(exc); \ + if (size < sizeof(x)) { \ + THROW(exc); \ + } \ strlcpy(buff, x, size); \ }) @@ -174,7 +184,9 @@ void number_to_string_indirect32(char *const dest, uint32_t const *const number) { check_null(dest); check_null(number); - if (buff_size < MAX_INT_DIGITS + 1) THROW(EXC_WRONG_LENGTH); // terminating null + if (buff_size < MAX_INT_DIGITS + 1) { + THROW(EXC_WRONG_LENGTH); // terminating null + } number_to_string(dest, *number); } @@ -183,8 +195,9 @@ void microtez_to_string_indirect(char *const dest, uint64_t const *const number) { check_null(dest); check_null(number); - if (buff_size < MAX_INT_DIGITS + sizeof(TICKER_WITH_SPACE) + 1) + if (buff_size < MAX_INT_DIGITS + sizeof(TICKER_WITH_SPACE) + 1) { THROW(EXC_WRONG_LENGTH); // + terminating null + decimal point + } microtez_to_string(dest, *number); } @@ -247,6 +260,8 @@ void copy_string(char *const dest, size_t const buff_size, char const *const src check_null(src); char const *const src_in = (char const *) PIC(src); // I don't care that we will loop through the string twice, latency is not an issue - if (strlen(src_in) >= buff_size) THROW(EXC_WRONG_LENGTH); + if (strlen(src_in) >= buff_size) { + THROW(EXC_WRONG_LENGTH); + } strlcpy(dest, src_in, buff_size); } diff --git a/src/ui.h b/src/ui.h index ca565078..05f30e02 100644 --- a/src/ui.h +++ b/src/ui.h @@ -21,4 +21,4 @@ void ux_empty_screen(void); /* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ void init_screen_stack(); /* User MUST call `init_screen_stack()` before calling this function for the first time. */ -void push_ui_callback(char *title, string_generation_callback cb, void *data); \ No newline at end of file +void push_ui_callback(char *title, string_generation_callback cb, void *data); diff --git a/src/ui_bagl.c b/src/ui_bagl.c index e70c5b0b..ee8289bf 100644 --- a/src/ui_bagl.c +++ b/src/ui_bagl.c @@ -239,8 +239,12 @@ void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { global.dynamic_display.formatter_index = 0; global.dynamic_display.current_state = STATIC_SCREEN; - if (ok_c) global.dynamic_display.ok_callback = ok_c; - if (cxl_c) global.dynamic_display.cxl_callback = cxl_c; + if (ok_c) { + global.dynamic_display.ok_callback = ok_c; + } + if (cxl_c) { + global.dynamic_display.cxl_callback = cxl_c; + } } void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c) { diff --git a/src/ui_sign_bagl.c b/src/ui_sign_bagl.c index ddcd7408..8d94eda1 100644 --- a/src/ui_sign_bagl.c +++ b/src/ui_sign_bagl.c @@ -22,7 +22,9 @@ __attribute__((noreturn)) void prompt_register_delegate(ui_callback_t const ok_cb, ui_callback_t const cxl_cb) { - if (!G.maybe_ops.is_valid) THROW(EXC_MEMORY_ERROR); + if (!G.maybe_ops.is_valid) { + THROW(EXC_MEMORY_ERROR); + } init_screen_stack(); push_ui_callback("Register", copy_string, "as delegate?"); diff --git a/src/ui_sign_nbgl.c b/src/ui_sign_nbgl.c index 33a10fae..41617d26 100644 --- a/src/ui_sign_nbgl.c +++ b/src/ui_sign_nbgl.c @@ -64,7 +64,9 @@ static void continue_light_callback(void) { } void prompt_register_delegate(ui_callback_t const ok_cb, ui_callback_t const cxl_cb) { - if (!G.maybe_ops.is_valid) THROW(EXC_MEMORY_ERROR); + if (!G.maybe_ops.is_valid) { + THROW(EXC_MEMORY_ERROR); + } transactionContext.ok_cb = ok_cb; transactionContext.cxl_cb = cxl_cb; diff --git a/test/README.md b/test/README.md index c9a18101..1874af45 100644 --- a/test/README.md +++ b/test/README.md @@ -3,14 +3,14 @@ The tests for the ledger are split into that of two types. 1) The apdu tests and 2) the flextesa tests. ### APDU tests -APDU tests use the ledgerblue python app to send bytes directly to the ledger. +APDU tests use the ledgerblue python app to send bytes directly to the ledger. They are split up into tests that run on the wallet app, and tests that run on the baking app. To execute them, simply run the various shell scripts found in `test/apdu-tests/` ### Flextesa These tests run a version of the tezos protocol in a small sandbox environment. It allows us to setup multiple accounts and run various scenarios in order to ensure that the ledger behaves appropriately. They can be run by using `test/run-flextesa-tests` and by -following the prompts. +following the prompts. #### Different sections of the ledger wallet test @@ -52,14 +52,14 @@ pack the repo into thunk form I have found that the best way to hack on the tests involve some sort of environement such as the following ``` cd nix/dep/flextesa-dev - nix-shell -p ocamlPackages.utop opam ocamlPackages.dune m4 pkgconfig gmp hidapi - opam init --root=. - eval $(opam env) + nix-shell -p ocamlPackages.utop opam ocamlPackages.dune m4 pkgconfig gmp hidapi + opam init --root=. + eval $(opam env) export PATH=_opam/bin:$PATH make build-dev-deps dune build src/bin_sandbox/main.exe ``` -I also usually tinker with the `test/run-flextesa-tests.sh` so that it gets the flextesa executable from `nix/dep/flextesa-dev/_build/default/src/bin_sandbox/main.exe` +I also usually tinker with the `test/run-flextesa-tests.sh` so that it gets the flextesa executable from `nix/dep/flextesa-dev/_build/default/src/bin_sandbox/main.exe` so that I can build and execute the test suite at a much faster rate than would otherwise be possible. ##### Flextesa Dbg module diff --git a/test/apdu-tests/apdu.sh b/test/apdu-tests/apdu.sh index 94925b56..688a0005 100755 --- a/test/apdu-tests/apdu.sh +++ b/test/apdu-tests/apdu.sh @@ -4,9 +4,9 @@ set -Eeuo pipefail root="$(git rev-parse --show-toplevel)" # Uncomment this line if you wish to send apdus to speculos (if you do, make sure to comment out the next line accordingly) -#nix-shell "$root/nix/ledgerblue.nix" -A shell --pure --run 'LEDGER_PROXY_ADDRESS=127.0.0.1 LEDGER_PROXY_PORT=9999 python -m ledgerblue.runScript' +#nix-shell "$root/nix/ledgerblue.nix" -A shell --pure --run 'LEDGER_PROXY_ADDRESS=127.0.0.1 LEDGER_PROXY_PORT=9999 python -m ledgerblue.runScript' -nix-shell "$root/nix/ledgerblue.nix" -A shell --pure --run 'python -m ledgerblue.runScript' +nix-shell "$root/nix/ledgerblue.nix" -A shell --pure --run 'python -m ledgerblue.runScript' ## NOTE: This additional option to the python script is useful "--apdu" for debugging, but is turned ## off to make it easier for QA to test