Skip to content

Commit

Permalink
Merge pull request #16 from Skydev0h/skydev/del-ext-safeguard
Browse files Browse the repository at this point in the history 
Added safeguard against deleting last extension with disallowed pubkey, fixed ext operation prefix clashing bug and adjusted code style
  • Loading branch information
@oleganza
oleganza authored Feb 23, 2024
2 parents 53cf00c + 51358ad commit fa1b372
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 22 deletions.
24 changes: 13 additions & 11 deletions contracts/wallet_v5.fc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,21 @@ cell verify_actions(cell c5) inline {
() dispatch_complex_request(slice cs) impure inline_ref {

;; Recurse into extended actions until we reach standard actions
while (cs~load_uint(1)) {
var is_add_ext = cs~check_and_remove_add_extension_prefix();
var is_del_ext = cs~check_and_remove_remove_extension_prefix();
while (cs~load_int(1)) {
var is_add_ext? = cs~check_and_remove_add_extension_prefix();
var is_del_ext? = is_add_ext? ? 0 : cs~check_and_remove_remove_extension_prefix();
;; Add/remove extensions
if ((is_add_ext) | (is_del_ext)) {
if (is_add_ext? | is_del_ext?) {
(int wc, int hash) = parse_std_addr(cs~load_msg_addr());
int packed_addr = pack_address((wc, hash) );

var ds = get_data().begin_parse();
var data_bits = ds~load_bits(size::stored_seqno + size::stored_subwallet + size::public_key);
var stored_seqno = data_bits.preload_int(size::stored_seqno);
var extensions = ds.preload_dict();

;; Add extension
if (is_add_ext) {
if (is_add_ext?) {
(extensions, int success?) = extensions.udict_add_builder?(256, packed_addr, begin_cell().store_int(wc,8));
throw_unless(39, success?);
} else
Expand All @@ -93,6 +94,7 @@ cell verify_actions(cell c5) inline {
{
(extensions, int success?) = extensions.udict_delete?(256, packed_addr);
throw_unless(40, success?);
throw_if(44, null?(extensions) & (stored_seqno < 0));
}

set_data(begin_cell()
Expand All @@ -101,11 +103,11 @@ cell verify_actions(cell c5) inline {
.end_cell());
}
elseif (cs~check_and_remove_set_signature_auth_allowed_prefix()) {
var allow = cs~load_uint(1);
var allow? = cs~load_int(1);
var ds = get_data().begin_parse();
var stored_seqno = ds~load_int(size::stored_seqno);
var immutable_tail = ds; ;; stored_subwallet ~ public_key ~ extensions
if (allow) {
if (allow?) {
;; allow
throw_unless(43, stored_seqno < 0);
;; Can't be disallowed with 0 because disallowing increments seqno
Expand Down Expand Up @@ -287,10 +289,10 @@ cell verify_actions(cell c5) inline {
;; - 0x6578746E "extn" authenticated by extension
;; - 0x73696E74 "sint" internal message authenticated by signature

(body, int is_extn) = check_and_remove_extn_prefix(body); ;; 0x6578746E ("extn")
(body, int is_extn?) = check_and_remove_extn_prefix(body); ;; 0x6578746E ("extn")

;; IFJMPREF because unconditionally returns inside
if (is_extn) { ;; "extn" authenticated by extension
if (is_extn?) { ;; "extn" authenticated by extension

;; Authenticate extension by its address.
int packed_sender_addr = pack_address(parse_std_addr(full_msg_slice~load_msg_addr())); ;; no PLDMSGADDR exists
Expand All @@ -310,8 +312,8 @@ cell verify_actions(cell c5) inline {
}

slice full_body = body;
(_, int is_sint) = check_and_remove_sint_prefix(body); ;; 0x73696E74 ("sint") - sign internal
return_unless(is_sint);
(_, int is_sint?) = check_and_remove_sint_prefix(body); ;; 0x73696E74 ("sint") - sign internal
return_unless(is_sint?);

;; Process the rest of the slice just like the signed request.
process_signed_request_from_internal_message(full_body);
Expand Down
8 changes: 4 additions & 4 deletions tests/wallet-v5-extensions.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,8 @@ describe('Wallet V5 extensions auth', () => {
const receipt = await walletV5.sendInternalMessageFromExtension(sender, {
value: toNano('0.1'),
body: packActionsList([
new ActionRemoveExtension(sender.address!),
new ActionSetSignatureAuthAllowed(true)
new ActionSetSignatureAuthAllowed(true),
new ActionRemoveExtension(sender.address!)
])
});

Expand Down Expand Up @@ -461,8 +461,8 @@ describe('Wallet V5 extensions auth', () => {
const receipt = await walletV5.sendInternalMessageFromExtension(sender, {
value: toNano('0.1'),
body: packActionsList([
new ActionRemoveExtension(sender.address!),
new ActionSetSignatureAuthAllowed(true)
new ActionSetSignatureAuthAllowed(true),
new ActionRemoveExtension(sender.address!)
])
});

Expand Down
29 changes: 25 additions & 4 deletions tests/wallet-v5-external.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -800,15 +800,14 @@ describe('Wallet V5 sign auth external', () => {
expect(contract_seqno).toEqual(seqno + 1);
});

it('Should add ext, disallow sign, remove ext, allow sign in one tx; send in other', async () => {
// N.B. Test that zero extensions do not prevent re-allowing the signature authentication
it('Should add ext, disallow sign, allow sign, remove ext in one tx; send in other', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

const actionsList = packActionsList([
new ActionAddExtension(testExtension),
new ActionSetSignatureAuthAllowed(false),
new ActionRemoveExtension(testExtension),
new ActionSetSignatureAuthAllowed(true)
new ActionSetSignatureAuthAllowed(true),
new ActionRemoveExtension(testExtension)
]);
const receipt = await walletV5.sendExternalSignedMessage(createBody(actionsList));
accountForGas(receipt.transactions);
Expand Down Expand Up @@ -856,6 +855,28 @@ describe('Wallet V5 sign auth external', () => {
expect(receiverBalanceAfter).toEqual(receiverBalanceBefore + forwardValue - fee);
});

it('Should fail removing last extension with signature auth disabled', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

const actionsList = packActionsList([
new ActionAddExtension(testExtension),
new ActionSetSignatureAuthAllowed(false),
new ActionRemoveExtension(testExtension)
]);
const receipt = await walletV5.sendExternalSignedMessage(createBody(actionsList));
accountForGas(receipt.transactions);

expect(
(
(receipt.transactions[0].description as TransactionDescriptionGeneric)
.computePhase as TransactionComputeVm
).exitCode
).toEqual(44);

const isSignatureAuthAllowed = await walletV5.getIsSignatureAuthAllowed();
expect(isSignatureAuthAllowed).toEqual(-1);
});

it('Should fail disallowing signature auth twice in tx', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

Expand Down
34 changes: 31 additions & 3 deletions tests/wallet-v5-internal.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1011,15 +1011,14 @@ describe('Wallet V5 sign auth internal', () => {
expect(contract_seqno).toEqual(seqno + 1);
});

it('Should add ext, disallow sign, remove ext, allow sign in one tx; send in other', async () => {
// N.B. Test that zero extensions do not prevent re-allowing the signature authentication
it('Should add ext, disallow sign, allow sign, remove ext in one tx; send in other', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

const actionsList = packActionsList([
new ActionAddExtension(testExtension),
new ActionSetSignatureAuthAllowed(false),
new ActionSetSignatureAuthAllowed(true),
new ActionRemoveExtension(testExtension),
new ActionSetSignatureAuthAllowed(true)
]);

const receipt = await walletV5.sendInternal(sender, {
Expand Down Expand Up @@ -1078,6 +1077,35 @@ describe('Wallet V5 sign auth internal', () => {
expect(receiverBalanceAfter).toEqual(receiverBalanceBefore + forwardValue - fee);
});

it('Should fail removing last extension with signature auth disabled', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

const actionsList = packActionsList([
new ActionAddExtension(testExtension),
new ActionSetSignatureAuthAllowed(false),
new ActionRemoveExtension(testExtension)
]);

const receipt = await walletV5.sendInternal(sender, {
sendMode: SendMode.PAY_GAS_SEPARATELY,
value: toNano(0.1),
body: createBody(actionsList)
});

expect(receipt.transactions.length).toEqual(2);
accountForGas(receipt.transactions);

expect(
(
(receipt.transactions[1].description as TransactionDescriptionGeneric)
.computePhase as TransactionComputeVm
).exitCode
).toEqual(44);

const isSignatureAuthAllowed = await walletV5.getIsSignatureAuthAllowed();
expect(isSignatureAuthAllowed).toEqual(-1);
});

it('Should fail disallowing signature auth twice in tx', async () => {
const testExtension = Address.parse('EQAvDfWFG0oYX19jwNDNBBL1rKNT9XfaGP9HyTb5nb2Eml6y');

Expand Down

0 comments on commit fa1b372

Please sign in to comment.