diff --git a/src/Smartpay/webhooks.ts b/src/Smartpay/webhooks.ts index 665cdf8..cae14f7 100644 --- a/src/Smartpay/webhooks.ts +++ b/src/Smartpay/webhooks.ts @@ -1,4 +1,4 @@ -import { createHmac } from 'crypto'; +import { createHmac, timingSafeEqual } from 'crypto'; import basex from 'base-x'; // eslint-disable-next-line import/no-extraneous-dependencies @@ -187,7 +187,20 @@ const webhooksMixin = (Base: T) => { secret, }); - return signature === calculatedSignature; + const signatureBuffer = Buffer.from(signature, 'utf8'); + const calculatedSignatureBuffer = Buffer.from( + calculatedSignature, + 'utf8' + ); + + // Compare in constant time to avoid leaking the expected signature + // through timing differences. timingSafeEqual requires equal-length + // inputs, so a length mismatch is an early (safe) rejection. + if (signatureBuffer.length !== calculatedSignatureBuffer.length) { + return false; + } + + return timingSafeEqual(signatureBuffer, calculatedSignatureBuffer); } static expressWebhookMiddleware(secret: string | Function) { diff --git a/test/unit.js b/test/unit.js index 57ecb82..1e66b72 100644 --- a/test/unit.js +++ b/test/unit.js @@ -155,6 +155,41 @@ test('Verify Webhook Signature Verification function', function testWebhookSigna t.ok(Smartpay.verifyWebhookSignature({ data, signature, secret })); }); +test('Reject invalid webhook signature', function testWebhookSignatureReject(t) { + t.plan(3); + + // eslint-disable-next-line max-len + const data = `1653028612220.{"id":"evt_test_dwPfFKu5iSEKyHR2LFj9Lx","object":"event","createdAt":1653028523052,"test":true,"eventData":{"type":"payment.created","version":"2022-02-18","data":{"id":"payment_test_35LxgmF5KM22XKG38BjpJg","object":"payment","test":true,"createdAt":1653028523020,"updatedAt":1653028523020,"amount":200,"currency":"JPY","order":"order_test_RiYq2rthzRHrkKVGeucSwn","reference":"order_ref_1234567","status":"processed","metadata":{}}}}`; + const secret = 'gybcsjixKyBW2d4z6iNPlaYzHUMtawnodwZt3W0q'; + const validSignature = + '68007ada8485ea0ceca7c5e879ae860a50412b7af95ab8e81b32a3e13f3c0832'; + + // Wrong signature of the same length must be rejected. + const wrongSignature = + `${validSignature.slice(0, -1)}0` === validSignature + ? `${validSignature.slice(0, -1)}1` + : `${validSignature.slice(0, -1)}0`; + + t.notOk( + Smartpay.verifyWebhookSignature({ data, signature: wrongSignature, secret }) + ); + + // A signature shorter than the expected one must be rejected (and must not + // throw from the constant-time comparison, which requires equal lengths). + t.notOk( + Smartpay.verifyWebhookSignature({ data, signature: 'deadbeef', secret }) + ); + + // A signature longer than the expected one must be rejected. + t.notOk( + Smartpay.verifyWebhookSignature({ + data, + signature: `${validSignature}00`, + secret, + }) + ); +}); + test('Test retry policy', async function testRetryPolicy(t) { t.plan(2);