Skip to content

Commit

Permalink
conway: review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@janmazak
janmazak committed Dec 16, 2023
1 parent 25f9281 commit c9a74f7
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 171 deletions.
4 changes: 0 additions & 4 deletions src/getPublicKeys.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ static void getPublicKeys_handleInitAPDU(const uint8_t* wireDataBuffer, size_t w
{
{
CHECK_STAGE(GET_KEYS_STAGE_INIT);

ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);
}
{
// parse data
Expand Down Expand Up @@ -186,8 +184,6 @@ void getPublicKeys_handleGetNextKeyAPDU(
{
CHECK_STAGE(GET_KEYS_STAGE_GET_KEYS);

ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);

VALIDATE(ctx->currentPath < ctx->numPaths, ERR_INVALID_STATE);

read_view_t view = make_read_view(wireDataBuffer, wireDataBuffer + wireDataSize);
Expand Down
80 changes: 54 additions & 26 deletions src/securityPolicy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ security_policy_t policyForSignTxCertificate(
}

// applicable to credentials that are witnessed in this tx
static bool forbiddenCredential(
static bool _forbiddenCredential(
sign_tx_signingmode_t txSigningMode,
const ext_credential_t* credential
)
Expand All @@ -1042,8 +1042,10 @@ static bool forbiddenCredential(
case EXT_CREDENTIAL_KEY_HASH:
// everything is expected to be governed by native scripts
return true;
default:
case EXT_CREDENTIAL_SCRIPT_HASH:
break;
default:
ASSERT(false);
}
break;

Expand All @@ -1060,15 +1062,16 @@ static bool forbiddenCredential(
// if the hash corresponds to some of his keys,
// and might inadvertently sign several certificates with a single witness
return true;
default:
case EXT_CREDENTIAL_KEY_PATH:
break;
default:
ASSERT(false);
}
break;

default:
// this should not be called in POOL_REGISTRATION signing modes
ASSERT(false);
break;
}

return false;
Expand All @@ -1079,17 +1082,20 @@ security_policy_t _policyForSignTxCertificateStakeCredential(
const ext_credential_t* stakeCredential
)
{
DENY_IF(forbiddenCredential(txSigningMode, stakeCredential));
DENY_IF(_forbiddenCredential(txSigningMode, stakeCredential));

switch (stakeCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isOrdinaryStakingKeyPath(&stakeCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&stakeCredential->keyPath));
break;

default:
case EXT_CREDENTIAL_KEY_HASH:
case EXT_CREDENTIAL_SCRIPT_HASH:
// the rest is OK, forbidden credentials have been dealt with above
break;

default:
ASSERT(false);
}

PROMPT();
Expand Down Expand Up @@ -1123,9 +1129,21 @@ security_policy_t policyForSignTxCertificateVoteDelegation(
const ext_drep_t* drep
)
{
// DRep can be anything, but if given by key path, it should be a valid path
if (drep->type == EXT_DREP_KEY_PATH) {
switch (drep->type) {
case EXT_DREP_KEY_PATH:
// DRep can be anything, but if given by key path, it should be a valid path
DENY_UNLESS(bip44_isDRepKeyPath(&drep->keyPath));
break;

case EXT_DREP_KEY_HASH:
case EXT_DREP_SCRIPT_HASH:
case EXT_DREP_ABSTAIN:
case EXT_DREP_NO_CONFIDENCE:
// nothing to deny
break;

default:
ASSERT(false);
}

return _policyForSignTxCertificateStakeCredential(txSigningMode, stakeCredential);
Expand All @@ -1137,17 +1155,21 @@ security_policy_t policyForSignTxCertificateCommitteeAuth(
const ext_credential_t* hotCredential
)
{
DENY_IF(forbiddenCredential(txSigningMode, coldCredential));
DENY_IF(_forbiddenCredential(txSigningMode, coldCredential));

switch (coldCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isCommitteeColdKeyPath(&coldCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&coldCredential->keyPath));
break;

default:
case EXT_CREDENTIAL_KEY_HASH:
case EXT_CREDENTIAL_SCRIPT_HASH:
// the rest is OK, forbidden credentials have been dealt with above
break;

default:
ASSERT(false);
}

switch (hotCredential->type) {
Expand All @@ -1173,17 +1195,21 @@ security_policy_t policyForSignTxCertificateCommitteeResign(
const ext_credential_t* coldCredential
)
{
DENY_IF(forbiddenCredential(txSigningMode, coldCredential));
DENY_IF(_forbiddenCredential(txSigningMode, coldCredential));

switch (coldCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isCommitteeColdKeyPath(&coldCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&coldCredential->keyPath));
break;

default:
case EXT_CREDENTIAL_KEY_HASH:
case EXT_CREDENTIAL_SCRIPT_HASH:
// the rest is OK, forbidden credentials have been dealt with above
break;

default:
ASSERT(false);
}

PROMPT();
Expand All @@ -1194,17 +1220,21 @@ security_policy_t policyForSignTxCertificateDRep(
const ext_credential_t* dRepCredential
)
{
DENY_IF(forbiddenCredential(txSigningMode, dRepCredential));
DENY_IF(_forbiddenCredential(txSigningMode, dRepCredential));

switch (dRepCredential->type) {
case EXT_CREDENTIAL_KEY_PATH:
DENY_UNLESS(bip44_isDRepKeyPath(&dRepCredential->keyPath));
DENY_IF(violatesSingleAccountOrStoreIt(&dRepCredential->keyPath));
break;

default:
case EXT_CREDENTIAL_KEY_HASH:
case EXT_CREDENTIAL_SCRIPT_HASH:
// the rest is OK, forbidden credentials have been dealt with above
break;

default:
ASSERT(false);
}

PROMPT();
Expand Down Expand Up @@ -1265,7 +1295,6 @@ security_policy_t policyForSignTxStakePoolRegistrationInit(

default:
ASSERT(false);
break;
}

DENY(); // should not be reached
Expand Down Expand Up @@ -1439,7 +1468,6 @@ security_policy_t policyForSignTxWithdrawal(
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
break;

Expand Down Expand Up @@ -1467,7 +1495,6 @@ security_policy_t policyForSignTxWithdrawal(
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
break;

Expand All @@ -1488,15 +1515,13 @@ security_policy_t policyForSignTxWithdrawal(
// in POOL_REGISTRATION signing modes, this certificate should have already been
// reported as invalid (only pool registration certificate is allowed)
ASSERT(false);
break;
}
break;

default:
// in POOL_REGISTRATION signing modes, non-zero number of withdrawals
// should have already been reported as invalid
ASSERT(false);
break;
}

DENY(); // should not be reached
Expand Down Expand Up @@ -1955,7 +1980,7 @@ security_policy_t policyForSignTxVotingProcedure(
break;

default:
break;
ASSERT(false);
}
break;

Expand All @@ -1971,8 +1996,13 @@ security_policy_t policyForSignTxVotingProcedure(
DENY();
break;

default:
case EXT_VOTER_COMMITTEE_HOT_SCRIPT_HASH:
case EXT_VOTER_DREP_SCRIPT_HASH:
// scripts are OK
break;

default:
ASSERT(false);
}
break;

Expand All @@ -1984,7 +2014,6 @@ security_policy_t policyForSignTxVotingProcedure(
default:
// this should not be called in POOL_REGISTRATION signing modes
ASSERT(false);
break;
}

SHOW();
Expand All @@ -1993,7 +2022,7 @@ security_policy_t policyForSignTxVotingProcedure(
// For treasury
security_policy_t policyForSignTxTreasury(
sign_tx_signingmode_t txSigningMode MARK_UNUSED,
uint64_t coin MARK_UNUSED
uint64_t treasury MARK_UNUSED
)
{
SHOW();
Expand All @@ -2002,7 +2031,7 @@ security_policy_t policyForSignTxTreasury(
// For donation
security_policy_t policyForSignTxDonation(
sign_tx_signingmode_t txSigningMode MARK_UNUSED,
uint64_t coin MARK_UNUSED
uint64_t donation MARK_UNUSED
)
{
SHOW();
Expand Down Expand Up @@ -2081,7 +2110,6 @@ security_policy_t policyForCVoteRegistrationPaymentDestination(

default:
ASSERT(false);
break;
}

DENY(); // should not be reached
Expand Down
4 changes: 2 additions & 2 deletions src/securityPolicy.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ security_policy_t policyForSignTxVotingProcedure(
ext_voter_t* voter
);

security_policy_t policyForSignTxTreasury(sign_tx_signingmode_t txSigningMode, uint64_t coin);
security_policy_t policyForSignTxTreasury(sign_tx_signingmode_t txSigningMode, uint64_t treasury);

security_policy_t policyForSignTxDonation(sign_tx_signingmode_t txSigningMode, uint64_t coin);
security_policy_t policyForSignTxDonation(sign_tx_signingmode_t txSigningMode, uint64_t donation);

security_policy_t policyForSignTxConfirm();

Expand Down
10 changes: 3 additions & 7 deletions src/signCVote.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ void signCVote_handleInitAPDU(
)
{
{
//sanity checks
// sanity checks
CHECK_STAGE(VOTECAST_STAGE_INIT);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);
}
{
TRACE_BUFFER(wireDataBuffer, wireDataSize);
Expand Down Expand Up @@ -127,9 +126,8 @@ void signCVote_handleVotecastChunkAPDU(
)
{
{
//sanity checks
// sanity checks
CHECK_STAGE(VOTECAST_STAGE_CHUNK);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);
}
{
read_view_t view = make_read_view(wireDataBuffer, wireDataBuffer + wireDataSize);
Expand Down Expand Up @@ -159,9 +157,8 @@ void signCVote_handleConfirmAPDU(
{
TRACE_STACK_USAGE();
{
//sanity checks
// sanity checks
CHECK_STAGE(VOTECAST_STAGE_CONFIRM);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);
}
{
// no data to receive
Expand Down Expand Up @@ -207,7 +204,6 @@ void signCVote_handleWitnessAPDU(
{
// sanity checks
CHECK_STAGE(VOTECAST_STAGE_WITNESS);
ASSERT(wireDataSize < BUFFER_SIZE_PARANOIA);
}

{
Expand Down
Loading

0 comments on commit c9a74f7

Please sign in to comment.