From da8296939f8767284b4581a91419013826877674 Mon Sep 17 00:00:00 2001 From: BernardOnuh Date: Wed, 1 Jul 2026 23:45:36 +0100 Subject: [PATCH 1/2] fix(#810): validate ratePerSecond/depositedAmount before BigInt coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createStream called BigInt() on ratePerSecond/depositedAmount before validation. Non-numeric input threw SyntaxError and missing input threw TypeError, but only RangeError was mapped to 400 — everything else fell through to 500. Adds presence + format validation before coercion. Closes #810 --- backend/src/controllers/stream.controller.ts | 48 ++++++++++++++++++-- backend/tests/stream.controller.test.ts | 40 ++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/backend/src/controllers/stream.controller.ts b/backend/src/controllers/stream.controller.ts index b7670e84..6d61a5f0 100644 --- a/backend/src/controllers/stream.controller.ts +++ b/backend/src/controllers/stream.controller.ts @@ -61,6 +61,35 @@ function sumStringI128(values: string[]): string { return total.toString(); } +/** + * Thrown when a request body field fails presence/format validation. Kept + * distinct from generic errors so createStream can reliably map it to a 400 + * response instead of falling through to the catch-all 500. + */ +class StreamValidationError extends Error { + constructor(message: string) { + super(message); + this.name = 'StreamValidationError'; + } +} + +/** + * Validate presence and integer format of a required i128-style field, then + * coerce it to a BigInt. Any missing value or conversion failure (SyntaxError + * from a non-numeric string, TypeError from undefined/null/objects, etc.) is + * normalized into a StreamValidationError so the caller can map it to 400. + */ +function parseRequiredBigIntField(fieldName: string, value: unknown): bigint { + if (value === undefined || value === null || value === '') { + throw new StreamValidationError(`Missing required field: ${fieldName}`); + } + try { + return BigInt(value as bigint | number | string | boolean); + } catch { + throw new StreamValidationError(`Invalid ${fieldName}: must be a valid integer`); + } +} + /** * Create a new stream (stub for on-chain indexing) */ @@ -70,8 +99,6 @@ export const createStream = async (req: Request, res: Response) => { const parsedStreamId = Number.parseInt(streamId, 10); const parsedStartTime = Number.parseInt(startTime, 10); - const parsedRatePerSecond = BigInt(ratePerSecond); - const parsedDepositedAmount = BigInt(depositedAmount); if (!Number.isFinite(parsedStreamId)) { return res.status(400).json({ error: 'Invalid streamId: must be a valid integer' }); @@ -81,6 +108,21 @@ export const createStream = async (req: Request, res: Response) => { return res.status(400).json({ error: 'Invalid startTime: must be a non-negative integer' }); } + // Presence/format validation happens here, before any BigInt coercion, + // so a malformed or missing numeric field always yields 400 rather than + // an uncaught SyntaxError/TypeError falling through to 500. + let parsedRatePerSecond: bigint; + let parsedDepositedAmount: bigint; + try { + parsedRatePerSecond = parseRequiredBigIntField('ratePerSecond', ratePerSecond); + parsedDepositedAmount = parseRequiredBigIntField('depositedAmount', depositedAmount); + } catch (validationError) { + if (validationError instanceof StreamValidationError) { + return res.status(400).json({ error: validationError.message }); + } + throw validationError; + } + if (parsedRatePerSecond <= 0n) { return res.status(400).json({ error: 'Invalid ratePerSecond: must be greater than zero' }); } @@ -774,4 +816,4 @@ export const resumeStream = async (req: Request, res: Response) => { logger.error('Error resuming stream:', error); return res.status(500).json({ error: 'Internal server error' }); } -}; \ No newline at end of file +}; diff --git a/backend/tests/stream.controller.test.ts b/backend/tests/stream.controller.test.ts index ef37fe1b..f449536f 100644 --- a/backend/tests/stream.controller.test.ts +++ b/backend/tests/stream.controller.test.ts @@ -89,6 +89,46 @@ describe('Stream Controller', () => { await createStream(req as Request, res as Response); expect(res.status).toHaveBeenCalledWith(400); }); + + it('should return 400 with a validation error for non-numeric ratePerSecond', async () => { + req.body.ratePerSecond = 'abc'; + await createStream(req as Request, res as Response); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.status).not.toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('ratePerSecond') }) + ); + }); + + it('should return 400 with a validation error for non-numeric depositedAmount', async () => { + req.body.depositedAmount = 'xyz'; + await createStream(req as Request, res as Response); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.status).not.toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('depositedAmount') }) + ); + }); + + it('should return 400, not 500, when ratePerSecond is missing', async () => { + delete req.body.ratePerSecond; + await createStream(req as Request, res as Response); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.status).not.toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('ratePerSecond') }) + ); + }); + + it('should return 400, not 500, when depositedAmount is missing', async () => { + delete req.body.depositedAmount; + await createStream(req as Request, res as Response); + expect(res.status).toHaveBeenCalledWith(400); + expect(res.status).not.toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ error: expect.stringContaining('depositedAmount') }) + ); + }); }); describe('listStreams', () => { From ba3f95539cfffb728f4427a4315b691428aaee18 Mon Sep 17 00:00:00 2001 From: "Bernard.O" <114490070+BernardOnuh@users.noreply.github.com> Date: Fri, 3 Jul 2026 09:51:10 +0100 Subject: [PATCH 2/2] Enhance JWT signature verification process Refactor signature verification to improve security and clarity. --- backend/src/middleware/auth.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/backend/src/middleware/auth.ts b/backend/src/middleware/auth.ts index 082d447d..9402a943 100644 --- a/backend/src/middleware/auth.ts +++ b/backend/src/middleware/auth.ts @@ -99,21 +99,28 @@ export function verifyJwt(token: string): { publicKey: string } | null { const [header, body, sig] = token.split('.'); if (!header || !body || !sig) return null; - // Verify signature + // Compute the expected signature and re-encode it as base64url so we can + // compare strings directly, rather than decoding the provided signature + // to bytes first. Decoding base64url to bytes silently discards the + // unused trailing bits of the final character (a 32-byte digest only + // uses 4 of the 6 bits in its last base64 character), which means a + // tampered last character can decode to the *same* bytes as the + // original and slip past a byte-level comparison undetected. const expected = crypto .createHmac('sha256', JWT_SECRET) .update(`${header}.${body}`) .digest(); + const expectedSig = b64url(expected); - let providedSig: Buffer; - try { - providedSig = Buffer.from(sig, 'base64url'); - } catch { - return null; - } + const providedSigBuf = Buffer.from(sig); + const expectedSigBuf = Buffer.from(expectedSig); - // Use timingSafeEqual to prevent timing attacks - if (providedSig.length !== expected.length || !crypto.timingSafeEqual(providedSig, expected)) { + // Use timingSafeEqual to prevent timing attacks. Lengths are checked + // first since timingSafeEqual throws on mismatched buffer lengths. + if ( + providedSigBuf.length !== expectedSigBuf.length || + !crypto.timingSafeEqual(providedSigBuf, expectedSigBuf) + ) { return null; }