Skip to content

Commit

Permalink
fix: audit
Browse files Browse the repository at this point in the history
  • Loading branch information
janmazak committed Mar 17, 2024
1 parent 0281c1f commit 5389ae9
Show file tree
Hide file tree
Showing 13 changed files with 37 additions and 19 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ We recommend using the containerized build. See [Getting started](doc/build.md)

### Loading the app

`make load`

Builds and loads the application into the connected device. Make sure to close the Ledger app on the device before running the command.

Most common reason for a failed loading is the app taking too much space. Check `make size` (should be below 140K or so).
We recommend using the [Ledger VS Code plugin](https://marketplace.visualstudio.com/items?itemName=LedgerHQ.ledger-dev-tools) to load the app on a device.

### Debug version

Expand All @@ -27,7 +23,7 @@ also comment out

DEFINES += RESET_ON_CRASH

and then run `make clean load`.
The debug version is too big to fit on Nano S, but works on Speculos.

### Setup

Expand Down Expand Up @@ -78,6 +74,10 @@ _Before merging a PR, one should make sure that:_
* `make clean load` runs without errors and warnings (except those reported for nanos-secure-sdk repo) for development build (see Debug version above)
* `make analyze` does not report errors or warnings

## Running tests

All the tests are initiated from the accompanying [ledgerjs package](https://github.com/vacuumlabs/ledgerjs-cardano-shelley) (see what [commands to run](https://github.com/vacuumlabs/ledgerjs-cardano-shelley?tab=readme-ov-file#tests)). You have to make sure that the version of ledgerjs correspond to the app version, otherwise some tests with fail (possibly resulting in odd errors) or test coverage will be incomplete.

## How to get a transaction body computed by Ledger

Ledger computes a rolling hash of the serialized transaction body, but the body itself is ordinarily not available. It is possible to acquire it from the development build by going through the following steps:
Expand Down
3 changes: 2 additions & 1 deletion fuzzing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ add_compile_definitions(
HAVE_ECDSA
HAVE_EDDSA
HAVE_HASH
HAVE_SHA224
HAVE_SHA256
HAVE_SHA3

Expand Down Expand Up @@ -181,4 +182,4 @@ foreach(harness IN LISTS harnesses)
./src/${harness}.c
)
target_link_libraries(${harness} PUBLIC cardano)
endforeach()
endforeach()
1 change: 1 addition & 0 deletions fuzzing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ Since there is an already existing corpus, to start fuzzing with it simply do `.


## Notes

For more context regarding fuzzing check out the app-boilerplate fuzzing [README.md](https://github.com/LedgerHQ/app-boilerplate/blob/master/fuzzing/README.md)
1 change: 1 addition & 0 deletions src/getPublicKeys.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ void getPublicKeys_handleAPDU(
bool isNewCall
)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

if (isNewCall) {
Expand Down
1 change: 1 addition & 0 deletions src/signCVote.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ void signCVote_handleAPDU(
bool isNewCall
)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

if (isNewCall) {
Expand Down
18 changes: 12 additions & 6 deletions src/signMsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -207,36 +207,40 @@ static size_t _createProtectedHeader(uint8_t* protectedHeaderBuffer, size_t maxS
{
size_t len = cbor_writeToken(CBOR_TYPE_MAP, 2, p, end - p);
p += len;
ASSERT(p < end);
}
{
size_t len = cbor_writeToken(CBOR_TYPE_UNSIGNED, 1, p, end - p);
p += len;
ASSERT(p < end);
}
{
size_t len = cbor_writeToken(CBOR_TYPE_NEGATIVE, -8, p, end - p);
p += len;
ASSERT(p < end);
}
{
size_t len = cbor_writeToken(CBOR_TYPE_TEXT, 7, p, end - p);
p += len;
ASSERT(p < end);
}

{
const char* text = "address";
const size_t len = strlen(text);
ASSERT(p + len < end);
memmove(p, text, len);
p += len;
ASSERT(p < end);
}
_prepareAddressField();
{
_prepareAddressField();

size_t len = cbor_writeToken(CBOR_TYPE_BYTES, ctx->addressFieldSize, p, end - p);
p += len;
}
{
ASSERT(p + ctx->addressFieldSize < end);
memmove(p, ctx->addressField, ctx->addressFieldSize);
p += ctx->addressFieldSize;
ASSERT(p < end);
}

const size_t protectedHeaderSize = p - protectedHeaderBuffer;
Expand All @@ -250,7 +254,7 @@ static size_t _createProtectedHeader(uint8_t* protectedHeaderBuffer, size_t maxS
}
} END_TRY;

return -1;
return SIZE_MAX;
}

static void signMsg_handleConfirmAPDU(const uint8_t* wireDataBuffer MARK_UNUSED, size_t wireDataSize)
Expand Down Expand Up @@ -283,8 +287,9 @@ static void signMsg_handleConfirmAPDU(const uint8_t* wireDataBuffer MARK_UNUSED,
ASSERT(written < maxWritten);
}
{
uint8_t protectedHeaderBuffer[100]; // address of max 57 bytes plus a couple of small items
uint8_t protectedHeaderBuffer[100] = {0}; // address of max 57 bytes plus a couple of small items
const size_t len = _createProtectedHeader(protectedHeaderBuffer, SIZEOF(protectedHeaderBuffer));
ASSERT(len < BUFFER_SIZE_PARANOIA);
written += cbor_writeToken(CBOR_TYPE_BYTES, len, sigStructure + written, maxWritten - written);
ASSERT(written + len < maxWritten);
memmove(sigStructure + written, protectedHeaderBuffer, len);
Expand Down Expand Up @@ -360,6 +365,7 @@ void signMsg_handleAPDU(
)
{
TRACE("P1 = 0x%x, P2 = 0x%x, isNewCall = %d", p1, p2, isNewCall);
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

VALIDATE(p2 == P2_UNUSED, ERR_INVALID_REQUEST_PARAMETERS);
Expand Down
3 changes: 2 additions & 1 deletion src/signMsg_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ void _displayMsgIntro(ui_callback_fn_t* callback)
ASSERT(strlen(l1) + 1 < SIZEOF(l1));

char l2[30] = {0};
snprintf(l2, SIZEOF(l2), "%u bytes", ctx->msgLength);
ASSERT(ctx->msgLength < UINT32_MAX);
snprintf(l2, SIZEOF(l2), "%u bytes", (uint32_t)ctx->msgLength);
ASSERT(strlen(l2) + 1 < SIZEOF(l2));

#ifdef HAVE_BAGL
Expand Down
6 changes: 3 additions & 3 deletions src/signMsg_ui.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#ifndef H_CARDANO_APP_SIGN_CVOTE_UI
#define H_CARDANO_APP_SIGN_CVOTE_UI
#ifndef H_CARDANO_APP_SIGN_MSG_UI
#define H_CARDANO_APP_SIGN_MSG_UI

#include "uiHelpers.h"

Expand Down Expand Up @@ -40,4 +40,4 @@ enum {

void signMsg_handleConfirm_ui_runStep();

#endif // H_CARDANO_APP_SIGN_CVOTE_UI
#endif // H_CARDANO_APP_SIGN_MSG_UI
1 change: 1 addition & 0 deletions src/signTxCVoteRegistration.c
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ bool signTxCVoteRegistration_isValidInstruction(uint8_t p2)

void signTxCVoteRegistration_handleAPDU(uint8_t p2, const uint8_t* wireDataBuffer, size_t wireDataSize)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

switch (p2) {
Expand Down
1 change: 1 addition & 0 deletions src/signTxMint.c
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ void signTxMint_init()

void signTxMint_handleAPDU(uint8_t p2, const uint8_t* wireDataBuffer, size_t wireDataSize)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

switch (p2) {
Expand Down
2 changes: 2 additions & 0 deletions src/signTxOutput.c
Original file line number Diff line number Diff line change
Expand Up @@ -1071,6 +1071,7 @@ bool signTxOutput_isValidInstruction(uint8_t p2)

void signTxOutput_handleAPDU(uint8_t p2, const uint8_t* wireDataBuffer, size_t wireDataSize)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

switch (p2) {
Expand Down Expand Up @@ -1132,6 +1133,7 @@ bool signTxCollateralOutput_isValidInstruction(uint8_t p2)

void signTxCollateralOutput_handleAPDU(uint8_t p2, const uint8_t* wireDataBuffer, size_t wireDataSize)
{
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

switch (p2) {
Expand Down
6 changes: 4 additions & 2 deletions src/signTxOutput_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,8 @@ void signTxOutput_handleDatumInline_ui_runStep()
size_t datumSize = subctx->stateData.datumRemainingBytes + subctx->stateData.datumChunkSize;
// datumSize with 6 digits fits on the screen, less than max tx size
// if more is needed, "bytes" can be replaced by "B" for those larger numbers
snprintf(l1, SIZEOF(l1), "Datum %u bytes", datumSize);
ASSERT(datumSize < UINT32_MAX);
snprintf(l1, SIZEOF(l1), "Datum %u bytes", (uint32_t)datumSize);
ASSERT(strlen(l1) + 1 < SIZEOF(l1));

char l2[20];
Expand Down Expand Up @@ -397,7 +398,8 @@ void handleRefScript_ui_runStep()
size_t scriptSize = subctx->stateData.refScriptRemainingBytes + subctx->stateData.refScriptChunkSize;
// scriptSize with 6 digits fits on the screen, less than max tx size
// if more is needed, "bytes" can be replaced by "B" for those larger numbers
snprintf(l1, SIZEOF(l1), "Script %u bytes", scriptSize);
ASSERT(scriptSize < UINT32_MAX);
snprintf(l1, SIZEOF(l1), "Script %u bytes", (uint32_t)scriptSize);
ASSERT(strlen(l1) + 1 < SIZEOF(l1));

char l2[20];
Expand Down
1 change: 1 addition & 0 deletions src/signTxPoolRegistration.c
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,7 @@ void signTxPoolRegistration_handleAPDU(uint8_t p2, const uint8_t* wireDataBuffer
{
TRACE_STACK_USAGE();
TRACE("p2 = 0x%x", p2);
ASSERT(wireDataBuffer != NULL);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

pool_registration_context_t* subctx = accessSubcontext();
Expand Down

0 comments on commit 5389ae9

Please sign in to comment.