Skip to content

fix(core): tighten isEvent type-guard against type confusion (#60)#86

Open
bnuckols13 wants to merge 1 commit into
hzrd149:masterfrom
bnuckols13:fix/isevent-type-confusion
Open

fix(core): tighten isEvent type-guard against type confusion (#60)#86
bnuckols13 wants to merge 1 commit into
hzrd149:masterfrom
bnuckols13:fix/isevent-type-confusion

Conversation

@bnuckols13

Copy link
Copy Markdown

Closes #60.

isEvent() guards most event handling in the library, but two of its checks read a property's length without first checking the value's type, so non-event objects pass the guard and reach code that assumes a well-formed NostrEvent.

Reproduced on master (98875b1f):

const valid = { id: "a".repeat(64), sig: "b".repeat(128), pubkey: "a".repeat(64), content: "", tags: [], created_at: 1 };

// an array of length 64 satisfies `event.id?.length === 64`
isEvent({ ...valid, id: new Array(64) }); // => true (should be false)

// a non-integer timestamp satisfies `created_at > 0`
isEvent({ ...valid, created_at: 0.5 });      // => true
isEvent({ ...valid, created_at: Infinity }); // => true

Fix (packages/core/src/helpers/event.ts): require id to be a string before trusting its length, and require created_at to be an integer.

-    event.id?.length === 64 &&
+    typeof event.id === "string" &&
+    event.id.length === 64 &&
     typeof event.sig === "string" &&
     typeof event.pubkey === "string" &&
     event.pubkey.length === 64 &&
     typeof event.content === "string" &&
     Array.isArray(event.tags) &&
     typeof event.created_at === "number" &&
+    Number.isInteger(event.created_at) &&
     event.created_at > 0

A regression test covering both bypasses (plus the valid and null/undefined cases) is added to packages/core/src/helpers/__tests__/events.test.ts, which previously had no coverage for isEvent. It fails on the unpatched guard and passes with the fix; the rest of the vitest suite is unchanged.

On sig length (issue point ②): I did not add event.sig.length === 128. isEvent's docstring states it does not validate the signature, and isValidNostrWebToken (packages/common) relies on that contract to check token structure before the signature is verified (its fixture carries a placeholder sig). Requiring a 128-character signature inside the structural guard breaks that usage, so signature length and hex-format validation are better placed in a signature-checking path than in isEvent. This keeps the change scoped to the type-confusion holes and leaves the guard's documented contract intact. Happy to add stricter hex/format validation in a follow-up if you'd prefer it live here.

)

isEvent validated `id` and `created_at` by length/comparison without
checking their type, so a non-string `id` of length 64 (e.g. an array)
and a non-integer `created_at` (0.5, Infinity) passed the guard and
reached code that assumes a well-formed NostrEvent. Require `id` to be a
string and `created_at` to be an integer, and add regression tests
(isEvent previously had none).

isEvent stays signature-agnostic per its docstring; sig-length policy
belongs in a verifying path, since isValidNostrWebToken relies on the
structural-only contract to check tokens before verification.
@changeset-bot

changeset-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: be94695

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SECURITY: isEvent() weak validation allows malformed events to pass type guard

1 participant