Skip to content

Commit 38d9134

Browse files
adamfraseradamfraser
authored
feat: Add functionality to validate authenticators added my clients (#335)
Authenticators can be easily misused. This code adds an `validateAuthenticator` function that does some rudimentary checks to ensure that users are adding a signatureVerification authenticator as part of any combination of authenticators that are added.
1 parent 8c07023 commit 38d9134

File tree

5 files changed

+244
-20
lines changed

5 files changed

+244
-20
lines changed

‎v4-client-js/__tests__/clients/composite-client.test.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
import { Authenticator, AuthenticatorType, Network } from '../../src';
2+
import { CompositeClient } from '../../src/clients/composite-client'
3+
4+
describe('CompositeClient', () => {
5+
describe('validateAuthenticators', () => {
6+
const network = Network.staging();
7+
8+
it('Validates top level AnyOf authenticators', async () => {
9+
const client = await CompositeClient.connect(network);
10+
const auth: Authenticator = {
11+
type: AuthenticatorType.ANY_OF,
12+
config: [
13+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
14+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
15+
],
16+
}
17+
expect(client.validateAuthenticator(auth)).toEqual(true);
18+
});
19+
20+
it('Fails top level AnyOf authenticators with non-signature nested authenticator', async () => {
21+
const client = await CompositeClient.connect(network);
22+
const auth: Authenticator = {
23+
type: AuthenticatorType.ANY_OF,
24+
config: [
25+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
26+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
27+
],
28+
}
29+
expect(client.validateAuthenticator(auth)).toEqual(false);
30+
});
31+
32+
it('Validates top level AllOf authenticators', async () => {
33+
const client = await CompositeClient.connect(network);
34+
const auth: Authenticator = {
35+
type: AuthenticatorType.ALL_OF,
36+
config: [
37+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
38+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
39+
],
40+
}
41+
expect(client.validateAuthenticator(auth)).toEqual(true);
42+
});
43+
44+
it('Fails top level AllOf authenticators, without signature verification', async () => {
45+
const client = await CompositeClient.connect(network);
46+
const auth: Authenticator = {
47+
type: AuthenticatorType.ALL_OF,
48+
config: [
49+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
50+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
51+
],
52+
}
53+
expect(client.validateAuthenticator(auth)).toEqual(false);
54+
});
55+
56+
it('Validates nested anyOf authenticators', async () => {
57+
const client = await CompositeClient.connect(network);
58+
const nestedAnyOf = {
59+
type: AuthenticatorType.ANY_OF,
60+
config: [
61+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
62+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
63+
]
64+
}
65+
const signatureVerification = {
66+
type: AuthenticatorType.SIGNATURE_VERIFICATION,
67+
config: ""
68+
}
69+
70+
const auth: Authenticator = {
71+
type: AuthenticatorType.ALL_OF,
72+
config: [signatureVerification, nestedAnyOf],
73+
};
74+
expect(client.validateAuthenticator(auth)).toEqual(true);
75+
});
76+
77+
it('Validates nested allOf authenticators', async () => {
78+
const client = await CompositeClient.connect(network);
79+
const nestedAllOf = {
80+
type: AuthenticatorType.ALL_OF,
81+
config: [
82+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
83+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
84+
]
85+
}
86+
const messageVerification = {
87+
type: AuthenticatorType.MESSAGE_FILTER,
88+
config: ""
89+
}
90+
91+
const auth: Authenticator = {
92+
type: AuthenticatorType.ALL_OF,
93+
config: [messageVerification, nestedAllOf],
94+
};
95+
expect(client.validateAuthenticator(auth)).toEqual(true);
96+
});
97+
98+
it('Fails nested anyOf signatureVerification authenticators', async () => {
99+
const client = await CompositeClient.connect(network);
100+
const nestedAnyOf = {
101+
type: AuthenticatorType.ANY_OF,
102+
config: [
103+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
104+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
105+
]
106+
}
107+
const messageVerification = {
108+
type: AuthenticatorType.MESSAGE_FILTER,
109+
config: ""
110+
}
111+
112+
const auth: Authenticator = {
113+
type: AuthenticatorType.ALL_OF,
114+
config: [messageVerification, nestedAnyOf],
115+
};
116+
expect(client.validateAuthenticator(auth)).toEqual(false);
117+
});
118+
119+
it('Fails nested anyOf authenticators', async () => {
120+
const client = await CompositeClient.connect(network);
121+
const nestedAnyOf = {
122+
type: AuthenticatorType.ANY_OF,
123+
config: [
124+
{ type: AuthenticatorType.MESSAGE_FILTER, config: "" },
125+
{ type: AuthenticatorType.SIGNATURE_VERIFICATION, config: "" },
126+
]
127+
}
128+
const messageVerification = {
129+
type: AuthenticatorType.MESSAGE_FILTER,
130+
config: ""
131+
}
132+
133+
const auth: Authenticator = {
134+
type: AuthenticatorType.ANY_OF,
135+
config: [messageVerification, nestedAnyOf],
136+
};
137+
expect(client.validateAuthenticator(auth)).toEqual(false);
138+
});
139+
});
140+
});

‎v4-client-js/examples/permissioned_keys_example.ts‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -47,31 +47,57 @@ async function test(): Promise<void> {
4747
await placeOrder(client, subaccount2, subaccount1, authenticatorID);
4848
}
4949

50-
async function removeAuthenticator(client: CompositeClient,subaccount: SubaccountInfo, id: Long): Promise<void> {
50+
async function removeAuthenticator(
51+
client: CompositeClient,
52+
subaccount: SubaccountInfo,
53+
id: Long,
54+
): Promise<void> {
5155
await client.removeAuthenticator(subaccount, id);
5256
}
5357

54-
async function addAuthenticator(client: CompositeClient, subaccount: SubaccountInfo, authedPubKey:string): Promise<void> {
55-
const subAuthenticators = [{
58+
async function addAuthenticator(
59+
client: CompositeClient,
60+
subaccount: SubaccountInfo,
61+
authedPubKey: string,
62+
): Promise<void> {
63+
const subAuth = [ {
5664
type: AuthenticatorType.SIGNATURE_VERIFICATION,
5765
config: authedPubKey,
5866
},
5967
{
60-
type: AuthenticatorType.MESSAGE_FILTER,
61-
config: toBase64(new TextEncoder().encode("/dydxprotocol.clob.MsgPlaceOrder")),
62-
},
68+
type: AuthenticatorType.ANY_OF,
69+
config: [
70+
{
71+
type: AuthenticatorType.MESSAGE_FILTER,
72+
config: toBase64(new TextEncoder().encode('/dydxprotocol.clob.MsgPlaceOrder')),
73+
},
74+
{
75+
type: AuthenticatorType.MESSAGE_FILTER,
76+
config: toBase64(new TextEncoder().encode('/dydxprotocol.clob.MsgPlaceOrder')),
77+
},
78+
]
79+
}
6380
];
6481

65-
const jsonString = JSON.stringify(subAuthenticators);
82+
const jsonString = JSON.stringify(subAuth);
6683
const encodedData = new TextEncoder().encode(jsonString);
6784

68-
await client.addAuthenticator(subaccount, AuthenticatorType.ALL_OF, encodedData);
85+
try {
86+
await client.addAuthenticator(subaccount, AuthenticatorType.ALL_OF, encodedData);
87+
} catch (error) {
88+
console.log(error.message);
89+
}
6990
}
7091

71-
async function placeOrder(client: CompositeClient, fromAccount: SubaccountInfo, forAccount: SubaccountInfo, authenticatorId: Long): Promise<void> {
92+
async function placeOrder(
93+
client: CompositeClient,
94+
fromAccount: SubaccountInfo,
95+
forAccount: SubaccountInfo,
96+
authenticatorId: Long,
97+
): Promise<void> {
7298
try {
73-
const side = OrderSide.BUY
74-
const price = Number("1000");
99+
const side = OrderSide.BUY;
100+
const price = Number('1000');
75101
const currentBlock = await client.validatorClient.get.latestBlockHeight();
76102
const nextValidBlockHeight = currentBlock + 5;
77103
const goodTilBlock = nextValidBlockHeight + 10;
@@ -94,7 +120,7 @@ async function placeOrder(client: CompositeClient, fromAccount: SubaccountInfo,
94120
{
95121
authenticators: [authenticatorId],
96122
accountForOrder: forAccount,
97-
}
123+
},
98124
);
99125
console.log('**Order Tx**');
100126
console.log(Buffer.from(tx.hash).toString('hex'));

‎v4-client-js/package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎v4-client-js/src/clients/composite-client.ts‎

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { TextDecoder } from 'util';
2+
13
import { EncodeObject } from '@cosmjs/proto-signing';
24
import { Account, GasPrice, IndexedTx, StdFee } from '@cosmjs/stargate';
35
import { Method } from '@cosmjs/tendermint-rpc';
@@ -18,6 +20,7 @@ import { bigIntToBytes } from '../lib/helpers';
1820
import { isStatefulOrder, verifyOrderFlags } from '../lib/validation';
1921
import { GovAddNewMarketParams, OrderFlags } from '../types';
2022
import {
23+
Authenticator,
2124
AuthenticatorType,
2225
Network,
2326
OrderExecution,
@@ -169,7 +172,7 @@ export class CompositeClient {
169172
broadcastMode,
170173
account,
171174
undefined,
172-
authenticators,
175+
authenticators,
173176
);
174177
}
175178

@@ -311,9 +314,10 @@ export class CompositeClient {
311314
): Promise<BroadcastTxAsyncResponse | BroadcastTxSyncResponse | IndexedTx> {
312315
// For permissioned orders, use the permissioning account details instead of the subaccount
313316
// This allows placing orders on behalf of another account when using permissioned keys
314-
const accountForOrder = permissionedKeysAccountAuth ? permissionedKeysAccountAuth.accountForOrder : subaccount;
317+
const accountForOrder = permissionedKeysAccountAuth
318+
? permissionedKeysAccountAuth.accountForOrder
319+
: subaccount;
315320
const msgs: Promise<EncodeObject[]> = new Promise((resolve, reject) => {
316-
317321
const msg = this.placeShortTermOrderMessage(
318322
accountForOrder,
319323
marketId,
@@ -1233,25 +1237,74 @@ export class CompositeClient {
12331237
gasAdjustment?: number,
12341238
memo?: string,
12351239
): Promise<BroadcastTxAsyncResponse | BroadcastTxSyncResponse | IndexedTx> {
1236-
return this.validatorClient.post.createMarketPermissionless(ticker, subaccount, broadcastMode, gasAdjustment, memo);
1240+
return this.validatorClient.post.createMarketPermissionless(
1241+
ticker,
1242+
subaccount,
1243+
broadcastMode,
1244+
gasAdjustment,
1245+
memo,
1246+
);
12371247
}
12381248

12391249
async addAuthenticator(
12401250
subaccount: SubaccountInfo,
12411251
authenticatorType: AuthenticatorType,
12421252
data: Uint8Array,
12431253
): Promise<BroadcastTxAsyncResponse | BroadcastTxSyncResponse | IndexedTx> {
1244-
return this.validatorClient.post.addAuthenticator(subaccount, authenticatorType, data)
1254+
// Validate the provided authenticators before sending to the validator
1255+
const authenticator: Authenticator = {
1256+
type: authenticatorType,
1257+
config: JSON.parse(new TextDecoder().decode(data)),
1258+
};
1259+
if (!this.validateAuthenticator(authenticator)) {
1260+
throw new Error(
1261+
'Invalid authenticator, please ensure the authenticator permissions are correct',
1262+
);
1263+
}
1264+
1265+
return this.validatorClient.post.addAuthenticator(subaccount, authenticatorType, data);
12451266
}
12461267

12471268
async removeAuthenticator(
12481269
subaccount: SubaccountInfo,
12491270
id: Long,
12501271
): Promise<BroadcastTxAsyncResponse | BroadcastTxSyncResponse | IndexedTx> {
1251-
return this.validatorClient.post.removeAuthenticator(subaccount, id)
1272+
return this.validatorClient.post.removeAuthenticator(subaccount, id);
12521273
}
12531274

1254-
async getAuthenticators(address: string): Promise<GetAuthenticatorsResponse>{
1275+
async getAuthenticators(address: string): Promise<GetAuthenticatorsResponse> {
12551276
return this.validatorClient.get.getAuthenticators(address);
12561277
}
1278+
1279+
validateAuthenticator(authenticator: Authenticator): boolean {
1280+
function checkAuthenticator(auth: Authenticator): boolean {
1281+
if (auth.type === AuthenticatorType.SIGNATURE_VERIFICATION) {
1282+
return true; // A SignatureVerification authenticator is safe.
1283+
}
1284+
1285+
if (!Array.isArray(auth.config)) {
1286+
return false; // Unsafe case: a non-array config for a composite authenticator
1287+
}
1288+
1289+
if (auth.type === AuthenticatorType.ANY_OF) {
1290+
// ANY_OF is safe only if ALL sub-authenticators return true
1291+
return auth.config.every((nestedAuth) => checkAuthenticator(nestedAuth));
1292+
}
1293+
1294+
if (auth.type === AuthenticatorType.ALL_OF) {
1295+
// ALL_OF is safe if at least one sub-authenticator returns true
1296+
return auth.config.some((nestedAuth) => checkAuthenticator(nestedAuth));
1297+
}
1298+
1299+
// If it's a base-case authenticator but not SignatureVerification, it's unsafe
1300+
return false;
1301+
}
1302+
1303+
// The top-level authenticator must pass validation
1304+
if (!checkAuthenticator(authenticator)) {
1305+
return false
1306+
}
1307+
1308+
return true;
1309+
}
12571310
}

‎v4-client-js/src/clients/constants.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,11 @@ export enum AuthenticatorType {
211211
SUBACCOUNT_FILTER = 'SubaccountFilter',
212212
}
213213

214+
export interface Authenticator {
215+
type: AuthenticatorType;
216+
config: string | Authenticator[];
217+
}
218+
214219
export enum TradingRewardAggregationPeriod {
215220
DAILY = 'DAILY',
216221
WEEKLY = 'WEEKLY',

0 commit comments

Comments
 (0)