Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
title: Reject ASCII-whitespace-only webhook secrets
pr_url: https://github.com/stripe/stripe-node/pull/2885
semver_level: patch
jira_tickets_closed:
- RUN_DEVSDK-3378
---

- Reject webhook signing secrets made entirely of ASCII whitespace instead of using them as HMAC keys.
3 changes: 2 additions & 1 deletion src/StripeEventNotificationHandler.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {Stripe} from './stripe.core.js';
import * as Events from './resources/V2/Core/Events.js';
import {WebhookHeader, WebhookPayload} from './Webhooks.js';
import {isBlankWebhookSecret} from './utils.js';

export interface UnhandledNotificationDetails {
isKnownEventType: boolean;
Expand Down Expand Up @@ -380,7 +381,7 @@ export class StripeEventNotificationHandler extends BaseEventNotificationHandler
fallbackCallback: FallbackCallback
) {
super(client, fallbackCallback);
if (!webhookSecret) {
if (isBlankWebhookSecret(webhookSecret)) {
throw new Error('webhookSecret must be a non-empty string');
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/Webhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ import {
} from './crypto/CryptoProvider.js';
import {PlatformFunctions} from './platform/PlatformFunctions.js';
import {Event} from './resources/Events.js';
import {maybeExtractFromCloudProviderEnvelope, parsePayload} from './utils.js';
import {
isBlankWebhookSecret,
maybeExtractFromCloudProviderEnvelope,
parsePayload,
} from './utils.js';

/**
* Value of the `stripe-signature` header from Stripe.
Expand Down Expand Up @@ -258,7 +262,7 @@ export function createWebhooks(
encodedHeader,
this.EXPECTED_SCHEME
);
if (!secret) {
if (isBlankWebhookSecret(secret)) {
throw new StripeSignatureVerificationError(header, payload, {
message:
'No webhook secret value was provided. It should start with `whsec_`',
Expand Down Expand Up @@ -304,7 +308,7 @@ export function createWebhooks(
encodedHeader,
this.EXPECTED_SCHEME
);
if (!secret) {
if (isBlankWebhookSecret(secret)) {
throw new StripeSignatureVerificationError(header, payload, {
message:
'No webhook secret value was provided. It should start with `whsec_`',
Expand Down
4 changes: 4 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,10 @@ export function parsePayload(
return JSON.parse(raw) as Record<string, unknown>;
}

export function isBlankWebhookSecret(secret: string): boolean {
return !secret || /^[ \t\r\n\f\v]*$/.test(secret);
}

export function maybeExtractFromCloudProviderEnvelope(
payload: string | Uint8Array
): Record<string, unknown> {
Expand Down
57 changes: 51 additions & 6 deletions test/StripeEventNotificationHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,57 @@ describe('StripeEventNotificationHandler', () => {

expect(errorThrown).to.be.true;
});

it('should reject blank webhook secrets', () => {
const blankSecrets = [
'',
' ',
'\t',
'\r',
'\n',
'\f',
'\v',
' \t\r\n\f\v',
];

for (const secret of blankSecrets) {
expect(() => {
stripe.notificationHandler(secret, async () => {});
}).to.throw('webhookSecret must be a non-empty string');
}
});

it('should preserve surrounding whitespace in the webhook secret', async () => {
const secret = ` \t${DUMMY_WEBHOOK_SECRET}\r\n`;
let fallbackCalled = false;
const handler = stripe.notificationHandler(secret, async () => {
fallbackCalled = true;
});
const header = stripe.webhooks.generateTestHeaderString({
payload: v1BillingMeterPayload,
secret,
});

await handler.handle(v1BillingMeterPayload, header);

expect(fallbackCalled).to.be.true;
});

it('should accept a non-breaking-space-only webhook secret', async () => {
const secret = '\u00a0';
let fallbackCalled = false;
const handler = stripe.notificationHandler(secret, async () => {
fallbackCalled = true;
});
const header = stripe.webhooks.generateTestHeaderString({
payload: v1BillingMeterPayload,
secret,
});

await handler.handle(v1BillingMeterPayload, header);

expect(fallbackCalled).to.be.true;
});
});

describe('registeredEventTypes', () => {
Expand Down Expand Up @@ -948,12 +999,6 @@ describe('StripeEventNotificationHandlerWithoutVerification', () => {
expect(typeof handler.handle).to.equal('function');
});

it('should throw when constructing the original handler with an empty webhookSecret', () => {
expect(() => {
stripe.notificationHandler('', async () => {});
}).to.throw(/webhookSecret must be a non-empty string/);
});

describe('preHandle', () => {
it('should run the registered handler when no preHandle hook is registered', async () => {
let callbackCalled = false;
Expand Down
55 changes: 45 additions & 10 deletions test/Webhook.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,25 +485,60 @@ function createWebhooksTestSuite(stripe) {
);
});

it('should raise a SignatureVerificationError when the signing secret is empty or nullish', async () => {
it('should raise a SignatureVerificationError when the signing secret is blank or nullish', async () => {
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret: SECRET,
});

const expectedMessage = /No webhook secret value was provided\. It should start with `whsec_`/;
const blankSecrets = [
'',
' ',
'\t',
'\r',
'\n',
'\f',
'\v',
' \t\r\n\f\v',
null,
undefined,
];

await Promise.all(
blankSecrets.map((secret) =>
expect(
verifyHeaderFn(EVENT_PAYLOAD_STRING, header, secret)
).to.be.rejectedWith(
StripeSignatureVerificationError,
expectedMessage
)
)
);
});

await expect(
verifyHeaderFn(EVENT_PAYLOAD_STRING, header, '')
).to.be.rejectedWith(StripeSignatureVerificationError, expectedMessage);
it('should preserve surrounding whitespace in the signing secret', async () => {
const secret = ` \t${SECRET}\r\n`;
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret,
});

await expect(
verifyHeaderFn(EVENT_PAYLOAD_STRING, header, null)
).to.be.rejectedWith(StripeSignatureVerificationError, expectedMessage);
expect(
await verifyHeaderFn(EVENT_PAYLOAD_STRING, header, secret, 0)
).to.equal(true);
});

await expect(
verifyHeaderFn(EVENT_PAYLOAD_STRING, header, undefined)
).to.be.rejectedWith(StripeSignatureVerificationError, expectedMessage);
it('should accept a non-breaking-space-only signing secret', async () => {
const secret = '\u00a0';
const header = stripe.webhooks.generateTestHeaderString({
payload: EVENT_PAYLOAD_STRING,
secret,
});

expect(
await verifyHeaderFn(EVENT_PAYLOAD_STRING, header, secret, 0)
).to.equal(true);
});

describe('custom CryptoProvider', () => {
Expand Down
Loading