fix(security): prevent NaN and negative pagination values in API routes#32
fix(security): prevent NaN and negative pagination values in API routes#32Shreyassp002 wants to merge 1 commit into
Conversation
Updated multiple API endpoints (payment links and transactions) to properly parse and validate limit and offset values from query parameters using isNaN() checks and enforcing safe defaults and bounds to prevent NaN values from reaching the database. Co-authored-by: Shreyassp002 <96625037+Shreyassp002@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⚡ Flash Review
See inline comments for details. ⚡ Powered by Flash Review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const supabase = createServerClient() | ||
| const { searchParams } = new URL(req.url) | ||
| const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 100) | ||
| const rawLimit = parseInt(searchParams.get('limit') || '50') |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bugs: The previous parseInt could result in NaN if the limit parameter was not a valid number (e.g., limit=abc), which would then propagate to Math.min(NaN, 100) resulting in NaN. This change correctly handles NaN values and enforces a minimum limit of 1, improving robustness and preventing invalid database queries. This is a good improvement.
| const limit = Math.min(parseInt(searchParams.get('limit') || '10'), 100) | ||
| const offset = parseInt(searchParams.get('offset') || '0') | ||
| const rawLimit = parseInt(searchParams.get('limit') || '10') | ||
| const limit = isNaN(rawLimit) || rawLimit < 1 ? 10 : Math.min(rawLimit, 100) |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bug: The previous parseInt for limit and offset could result in NaN if the query parameters were non-numeric (e.g., limit=abc). Passing NaN to Supabase queries for pagination can lead to unexpected errors or incorrect results.
Fix: Explicitly checking isNaN and providing default values, along with range validation (rawLimit < 1, rawOffset < 0), significantly improves the robustness of the API. This prevents potential query failures and ensures valid pagination parameters are always used.
|
Closing as duplicate of #31 which has been merged. |
Understood. Acknowledging that this work is now obsolete and stopping work on this task. |
1 participant
Category: Security
Priority: P1
What: Fixed API pagination to validate limit and offset bounds and fallback if they are NaN.
Why: If a user sends a string like ?limit=abc, parseInt returns NaN, which propagates to Supabase .range(), leading to 500 Internal Server Errors or unbounded queries that could act as a DoS vector.
Impact: Safely validates and constrains user input before it hits the database.
Verification: Ran npm run lint and NEXT_PUBLIC_WALLET_CONNECT_PROJECT_ID=dummy npm run build.
PR created automatically by Jules for task 6377543979634899 started by @Shreyassp002