Skip to content

fix dataitem verification #3

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix dataitem verification #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
"tslib": "^2.4.0",
"tsutils": "~3.21.0",
"typescript": "^4.9.3",
"webpack": "^5.75.0",
"webpack": "^5.94.0",
"webpack-bundle-analyzer": "^4.8.0",
"webpack-cli": "^5.0.0"
},
Expand All @@ -137,11 +137,11 @@
"@ethersproject/transactions": "^5.7.0",
"@ethersproject/wallet": "^5.7.0",
"@noble/ed25519": "^1.6.1",
"arweave": "^1.15.1",
"arweave": "^1.15.5",
"base64url": "^3.0.1",
"bs58": "^4.0.1",
"keccak": "^3.0.2",
"secp256k1": "^5.0.0"
"secp256k1": "^5.0.1"
},
"optionalDependencies": {
"@randlabs/myalgo-connect": "^1.1.2",
Expand Down
48 changes: 36 additions & 12 deletions src/DataItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import type { Base64URLString } from "./types";

export const MIN_BINARY_SIZE = 80;
export const MAX_TAG_BYTES = 4096;
export const MAX_TAGS = 128;
export const MAX_TAG_KEY_LENGTH = 1024;
export const MAX_TAG_VALUE_LENGTH = 3072;

export class DataItem implements BundleItem {
private readonly binary: Buffer;
Expand Down Expand Up @@ -211,33 +214,54 @@ export class DataItem implements BundleItem {
}
const item = new DataItem(buffer);
const sigType = item.signatureType;
const tagsStart = item.getTagsStart();

const targetStart = item.getTargetStart();
const targetPresent = buffer[targetStart] === 1;
let anchorStart = targetStart + (targetPresent ? 33 : 1);
const anchorPresent = buffer[anchorStart] === 1;
const anchor = anchorPresent ? buffer.subarray(anchorStart + 1, anchorStart + 33) : Buffer.alloc(0);
if (anchor.length > 32) {
return false;
}
anchorStart += anchorPresent ? 33 : 1;
const tagsStart = anchorStart;
const numberOfTags = byteArrayToLong(buffer.subarray(tagsStart, tagsStart + 8));
if (numberOfTags > MAX_TAGS) {
return false;
}
const numberOfTagBytesArray = buffer.subarray(tagsStart + 8, tagsStart + 16);
const numberOfTagBytes = byteArrayToLong(numberOfTagBytesArray);

if (numberOfTagBytes > MAX_TAG_BYTES) return false;

if (numberOfTags > 0) {
try {
const tags: { name: string; value: string }[] = deserializeTags(
Buffer.from(buffer.subarray(tagsStart + 16, tagsStart + 16 + numberOfTagBytes)),
);

if (tags.length !== numberOfTags) {
return false;
}
} catch (e) {
for (const t of tags) {
if (!t.name || !t.value) {
return false;
}
const nameLen = Buffer.byteLength(t.name);
const valueLen = Buffer.byteLength(t.value);
if (nameLen === 0 || nameLen > MAX_TAG_KEY_LENGTH) {
return false;
}
if (valueLen === 0 || valueLen > MAX_TAG_VALUE_LENGTH) {
return false;
}
Comment on lines +247 to +252
Copy link

@fedellen fedellen Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this does respect the documented spec, there are existing apps and integrations using empty strings on name and value through Turbo

Suggested change
if (nameLen === 0 || nameLen > MAX_TAG_KEY_LENGTH) {
return false;
}
if (valueLen === 0 || valueLen > MAX_TAG_VALUE_LENGTH) {
return false;
}
if nameLen > MAX_TAG_KEY_LENGTH) {
return false;
}
if (valueLen > MAX_TAG_VALUE_LENGTH) {
return false;
}

Copy link
Author

@nikooo777 nikooo777 Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the input.
Apparently there are more instances of other projects violating the spec, so I think the only real change that stands a chance of getting eventually merged is going to be one where we simply check that there are at most 128 tags and each tag (name+val) is 4KB at most.

}
} catch (err) {
return false;
}
}

// eslint-disable-next-line @typescript-eslint/naming-convention
const Signer = indexToType[sigType];

const signatureHash = createHash("sha256").update(item.rawSignature).digest();
if (!signatureHash.equals(item.rawId)) {
return false;
}
const signatureData = await getSignatureData(item);
return await Signer.verify(item.rawOwner, signatureData, item.rawSignature);
const signer = indexToType[sigType];
return await signer.verify(item.rawOwner, signatureData, item.rawSignature);
}

public async getSignatureData(): Promise<Uint8Array> {
Expand Down
Loading