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 all 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
5 changes: 5 additions & 0 deletions .changeset/lemon-rockets-do.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@xmtp/frames-validator": patch
---

Fix V3 frames validation
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.30",
"@xmtp/xmtp-js": "^12.0.0",
"ethers": "^6.13.1",
"fast-glob": "^3.3.2",
Expand Down
6 changes: 3 additions & 3 deletions packages/frames-client/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
expect(signedPayloadProto.actionBody).toBeDefined();

if (isV3FramesSigner(signer)) {
expect(signedPayloadProto.signature).toBeUndefined();

Check warning on line 96 in packages/frames-client/src/client.test.ts

View workflow job for this annotation

GitHub Actions / Lint

`signature` is deprecated
expect(signedPayloadProto.signedPublicKeyBundle).toBeUndefined();
} else {
expect(signedPayloadProto.signature).toBeDefined();
Expand Down Expand Up @@ -133,7 +133,7 @@
// Will add E2E tests back once we have Frames deployed with the new schema
const worksE2E = (framesClient: FramesClient) => async () => {
const frameUrl =
"https://fc-polls-five.vercel.app/polls/01032f47-e976-42ee-9e3d-3aac1324f4b8";
"https://fc-polls-five.vercel.app/polls/03710836-bc1d-4921-9e24-89d82015c53b";
const metadata = await framesClient.proxy.readMetadata(frameUrl);
expect(metadata).toBeDefined();
expect(metadata.frameInfo).toMatchObject({
Expand All @@ -150,10 +150,10 @@
},
image: {
content:
"https://fc-polls-five.vercel.app/api/image?id=01032f47-e976-42ee-9e3d-3aac1324f4b8",
"https://fc-polls-five.vercel.app/api/image?id=03710836-bc1d-4921-9e24-89d82015c53b",
},
postUrl:
"https://fc-polls-five.vercel.app/api/vote?id=01032f47-e976-42ee-9e3d-3aac1324f4b8",
"https://fc-polls-five.vercel.app/api/vote?id=03710836-bc1d-4921-9e24-89d82015c53b",
});
const signedPayload = await framesClient.signFrameAction({
frameUrl,
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.30",
"@xmtp/proto": "^3.72.3",
"uint8array-extras": "^1.4.0",
"viem": "^2.16.5"
Expand Down
22 changes: 17 additions & 5 deletions packages/frames-validator/src/validation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { sha256 } from "@noble/hashes/sha256";
import { Client, getInboxIdForAddress, type XmtpEnv } from "@xmtp/node-sdk";
import { Client, type XmtpEnv } from "@xmtp/node-sdk";
import { fetcher, frames, type publicKey, type signature } from "@xmtp/proto";
import { uint8ArrayToHex } from "uint8array-extras";
import type {
Expand Down Expand Up @@ -28,7 +28,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,9 +48,21 @@ 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(
codabrink marked this conversation as resolved.
Show resolved Hide resolved
inboxId,
installationId,
{ env },
);
if (!authorized) {
throw new Error("Installation not a member of association state");
}

const isMember = await Client.isAddressAuthorized(inboxId, walletAddress, {
env,
});

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

const digest = sha256(actionBodyBytes);
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.30"
"@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.30"
"@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.28":
version: 0.0.28
resolution: "@xmtp/node-bindings@npm:0.0.28"
checksum: 10/0ceea72582926dcce03c8e3839b7101d7ce4aef56a55b46421c2e46e96f65c90a224b1649ff765c1f3cb1e2e21a730d1891bb369260c94712066b85d358fb6f4
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.30, @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