Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static Frames Validation Security Fix #736

Merged
merged 30 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/frames-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"@open-frames/types": "^0.1.1",
"@rollup/plugin-terser": "^0.4.4",
"@rollup/plugin-typescript": "^12.1.1",
"@xmtp/node-sdk": "^0.0.27",
"@xmtp/node-sdk": "^0.0.28",
"@xmtp/xmtp-js": "^12.0.0",
"ethers": "^6.13.1",
"fast-glob": "^3.3.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/frames-validator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"dependencies": {
"@noble/curves": "^1.3.0",
"@noble/hashes": "^1.4.0",
"@xmtp/node-sdk": "^0.0.27",
"@xmtp/node-sdk": "^0.0.28",
"@xmtp/proto": "^3.72.3",
"uint8array-extras": "^1.4.0",
"viem": "^2.16.5"
Expand Down
20 changes: 15 additions & 5 deletions packages/frames-validator/src/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type * from "./types.js";
const { b64Decode } = fetcher;

export async function validateFramesPost(
client: Client,
codabrink marked this conversation as resolved.
Show resolved Hide resolved
data: XmtpOpenFramesRequest,
env?: XmtpEnv,
): Promise<XmtpValidationResponse> {
Expand All @@ -28,7 +29,7 @@ export async function validateFramesPost(
actionBodyBytes,
signature,
signedPublicKeyBundle,
installationId, // not necessary
installationId,
codabrink marked this conversation as resolved.
Show resolved Hide resolved
installationSignature,
inboxId,
} = deserializeProtoMessage(messageBytes);
Expand All @@ -48,15 +49,24 @@ export async function validateFramesPost(
}
} else {
// make sure inbox IDs match
const addressInboxId = await getInboxIdForAddress(walletAddress, env);
if (inboxId !== addressInboxId) {
throw new Error("Invalid inbox ID");
const authorized = await client.isInstallationAuthorized(
inboxId,
installationId,
);
if (!authorized) {
throw new Error("Installation not a member of association state");
}

const isMember = await client.isAddressAuthorized(inboxId, walletAddress);

if (!isMember) {
throw new Error("Unable to associate wallet address with inbox");
}

const digest = sha256(actionBodyBytes);

// make sure installation signature is valid
const valid = Client.verifySignedWithPublicKey(
const valid = client.verifySignedWithPublicKey(
uint8ArrayToHex(digest),
installationSignature,
installationId,
Expand Down
28 changes: 14 additions & 14 deletions sdks/node-sdk/src/Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,6 @@ export class Client {
return this.#innerClient.isRegistered();
}

async isAddressAuthorized(
inboxId: string,
address: string,
): Promise<boolean> {
return this.#innerClient.isAddressAuthorized(inboxId, address);
}

async isInstallationAuthorized(
inboxId: string,
installationId: Uint8Array,
): Promise<boolean> {
return this.#innerClient.isInstallationAuthorized(inboxId, installationId);
}

async #createInboxSignatureText() {
try {
const signatureText = await this.#innerClient.createInboxSignatureText();
Expand Down Expand Up @@ -283,6 +269,20 @@ export class Client {
await this.#applySignatures();
}

async isAddressAuthorized(
inboxId: string,
address: string,
): Promise<boolean> {
return this.#innerClient.isAddressAuthorized(inboxId, address);
}

async isInstallationAuthorized(
inboxId: string,
installationId: Uint8Array,
): Promise<boolean> {
return this.#innerClient.isInstallationAuthorized(inboxId, installationId);
}

async removeAccount(accountAddress: string) {
const signatureText =
await this.#removeAccountSignatureText(accountAddress);
Expand Down
26 changes: 3 additions & 23 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5033,7 +5033,7 @@ __metadata:
"@open-frames/types": "npm:^0.1.1"
"@rollup/plugin-terser": "npm:^0.4.4"
"@rollup/plugin-typescript": "npm:^12.1.1"
"@xmtp/node-sdk": "npm:^0.0.27"
"@xmtp/node-sdk": "npm:^0.0.28"
"@xmtp/proto": "npm:^3.72.3"
"@xmtp/xmtp-js": "npm:^12.0.0"
ethers: "npm:^6.13.1"
Expand Down Expand Up @@ -5065,7 +5065,7 @@ __metadata:
"@rollup/plugin-typescript": "npm:^12.1.1"
"@types/bl": "npm:^5.1.4"
"@xmtp/frames-client": "npm:^1.0.0"
"@xmtp/node-sdk": "npm:^0.0.27"
"@xmtp/node-sdk": "npm:^0.0.28"
"@xmtp/proto": "npm:^3.72.3"
"@xmtp/xmtp-js": "npm:^12.1.0"
ethers: "npm:^6.10.0"
Expand All @@ -5079,34 +5079,14 @@ __metadata:
languageName: unknown
linkType: soft

"@xmtp/node-bindings@npm:^0.0.22":
version: 0.0.22
resolution: "@xmtp/node-bindings@npm:0.0.22"
checksum: 10/e8668b2fd30041dff8671625c13d49245d421ac31c0669b9be5365b5f22bf3c28f4d0cc12015a2710b90136423828bcdb1a99a10b37d4d5ac51e3ac47228b960
languageName: node
linkType: hard

"@xmtp/node-bindings@npm:^0.0.25":
version: 0.0.25
resolution: "@xmtp/node-bindings@npm:0.0.25"
checksum: 10/336f9b2a4a8b2781f6c53a7dd444428893f5e04a927350c82fcb79e37e0cc69abcb86406ffedeaa28115f1b57801890309546a10c9e806e3ed81b18b9d8f1431
languageName: node
linkType: hard

"@xmtp/node-sdk@npm:^0.0.27":
version: 0.0.27
resolution: "@xmtp/node-sdk@npm:0.0.27"
dependencies:
"@xmtp/content-type-group-updated": "npm:^1.0.1"
"@xmtp/content-type-primitives": "npm:^1.0.3"
"@xmtp/content-type-text": "npm:^1.0.1"
"@xmtp/node-bindings": "npm:^0.0.22"
"@xmtp/proto": "npm:^3.72.3"
checksum: 10/9937c77d4bd3f3ed8df2f9938e940e2c21a7c8e8f06506d227bb97939f8294e0ae6ec6bc1498f528274257d61f9142e38639aa123c3ddf086a98d9cd041c0161
languageName: node
linkType: hard

"@xmtp/node-sdk@workspace:sdks/node-sdk":
"@xmtp/node-sdk@npm:^0.0.28, @xmtp/node-sdk@workspace:sdks/node-sdk":
version: 0.0.0-use.local
resolution: "@xmtp/node-sdk@workspace:sdks/node-sdk"
dependencies:
Expand Down
Loading