fix: razorpay signature verification#351
Conversation
|
@Akhilesh29 is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughUpdated the payment verification method to streamline buffer creation by directly passing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/services/payment.service.ts`:
- Around line 127-130: The signature comparison currently passes hex strings
into Buffer.from without the 'hex' encoding and can throw a RangeError when
lengths differ; update the verification logic (where crypto.timingSafeEqual is
used with signature and generatedSignatureHex) to first create buffers by
decoding hex (Buffer.from(signature, 'hex') and
Buffer.from(generatedSignatureHex, 'hex')), check that the two buffers have
equal lengths and return false if they don't, and only then call
crypto.timingSafeEqual to compare the buffers; ensure any invalid-hex input is
handled (e.g., return false) rather than relying on catching the timingSafeEqual
exception.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9cc9f165-16dc-4243-840e-1ae1596828fd
📒 Files selected for processing (1)
apps/api/src/services/payment.service.ts
| return crypto.timingSafeEqual( | ||
| Buffer.from(signature), | ||
| Buffer.from(generatedSignatureHex) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
according to the current node.js docs, what encoding does Buffer.from(string)use when no encoding is provided, and doescrypto.timingSafeEqualthrow when the twoBuffer inputs have different byte lengths?
💡 Result:
According to the current Node.js documentation (v25.8.1), Buffer.from(string) with no encoding provided uses UTF-8 encoding. This is demonstrated by the example Buffer.from('tést') creating a Buffer with UTF-8-encoded bytes [0x74, 0xc3, 0x a9, 0x73, 0x74]. crypto.timingSafeEqual throws an error (specifically ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH, a RangeError) when the two Buffer inputs have different byte lengths. The documentation states that the inputs must have the same byte length, and an error is thrown if they differ.
Citations:
- 1: https://nodejs.org/api/buffer.html
- 2: https://nodejs.org/api/crypto.html
- 3: https://nodejs.cn/api/crypto/crypto_timingsafeequal_a_b.html
- 4: https://docs.deno.com/api/node/crypto/~/timingSafeEqual
- 5: https://runebook.dev/en/articles/node/errors/err_crypto_timing_safe_equal_length
- 6: nodejs/node@588b388181
Decode signature inputs as hex before comparison.
The current code passes hex strings to Buffer.from() without the "hex" encoding parameter, which treats them as UTF-8 text instead of decoding the hex bytes. Additionally, when the hex strings differ in length, crypto.timingSafeEqual throws a RangeError (as per Node.js docs), causing the exception to be caught and logged as an error. Pre-decode both inputs as hex and check length equality before calling timingSafeEqual to handle invalid signatures on the normal path without triggering exceptions.
suggested fix
- return crypto.timingSafeEqual(
- Buffer.from(signature),
- Buffer.from(generatedSignatureHex)
- );
+ const signatureBuffer = Buffer.from(signature, "hex");
+ const generatedSignatureBuffer = Buffer.from(
+ generatedSignatureHex,
+ "hex"
+ );
+
+ if (signatureBuffer.length !== generatedSignatureBuffer.length) {
+ return false;
+ }
+
+ return crypto.timingSafeEqual(
+ signatureBuffer,
+ generatedSignatureBuffer
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/services/payment.service.ts` around lines 127 - 130, The
signature comparison currently passes hex strings into Buffer.from without the
'hex' encoding and can throw a RangeError when lengths differ; update the
verification logic (where crypto.timingSafeEqual is used with signature and
generatedSignatureHex) to first create buffers by decoding hex
(Buffer.from(signature, 'hex') and Buffer.from(generatedSignatureHex, 'hex')),
check that the two buffers have equal lengths and return false if they don't,
and only then call crypto.timingSafeEqual to compare the buffers; ensure any
invalid-hex input is handled (e.g., return false) rather than relying on
catching the timingSafeEqual exception.
Summary by CodeRabbit