Skip to content

Conversation

@ajmallesh
Copy link
Collaborator

@ajmallesh ajmallesh commented Oct 7, 2025

Test PR: Per-PR SAST Engine Validation

This PR introduces 2 new injection vulnerabilities, 1 security fix, and 2 precision test cases to benchmark the per-PR SAST analysis engine.

Changes Made

🔴 NEW Vulnerabilities (2)

  1. routes/orderSearch.ts - NoSQL Injection (new file + route registered)

    • MongoDB $where operator with string interpolation
    • Route: GET /rest/orders/search
    • Source: req.query.email → Sink: ordersCollection.find()
  2. routes/search.ts - SQL Injection REGRESSION

    • Removed length validation (was substring(0,200), now unlimited)
    • Makes existing vulnerability worse - tests sanitization tracking

⚫ PRECISION Test Cases (Must NOT Detect - 2)

  1. routes/coupon.ts - SQL Injection Pattern in Dead Code

    • Function lookupCouponByCode() contains vulnerable pattern
    • BUT: Never registered as route - unreachable from HTTP requests
    • Purpose: Tests data flow analysis vs pattern matching
  2. routes/basket.ts - SQL Injection Pattern in Dead Code

    • Function findBasketByOwner() contains vulnerable pattern
    • BUT: Never registered as route - unreachable from HTTP requests
    • Purpose: Tests data flow analysis vs pattern matching

🟢 FIXED Vulnerability (1)

  1. routes/trackOrder.ts - NoSQL Injection REMEDIATED
    • Replaced unsafe $where operator with parameterized query
    • Before: { $where: \this.orderId === '${id}'` }`
    • After: { orderId: id }

Expected SAST Results

  • NEW: 2 vulnerabilities (orderSearch, search regression)
  • UNCHANGED: 5-8 baseline vulnerabilities (login.ts, updateProductReviews.ts, etc.)
  • FIXED: 1 vulnerability (trackOrder.ts)
  • FALSE POSITIVES: 0 (coupon.ts and basket.ts should NOT be detected - dead code)

What This Tests

Classification: NEW vs UNCHANGED vs FIXED
Precision: True data flow analysis (not just pattern matching) - 12 pts of grading
Performance: ~5-10 min speedup from skipped agents
Regression Detection: Sanitization degradation tracking

ajmallesh and others added 4 commits October 6, 2025 17:40
Introduces test vulnerabilities to validate per-PR SAST engine:

1. SQL injection in coupon lookup (routes/coupon.ts:45)
   - New function lookupCouponByCode() with template literal injection
   - Source: req.query.code → Sink: sequelize.query()

2. NoSQL injection in order search (routes/orderSearch.ts:16-18)
   - New file with MongoDB $where operator injection
   - Source: req.query.email → Sink: ordersCollection.find()

3. SQL injection regression in product search (routes/search.ts:24)
   - Removed length validation (was substring(0,200), now unlimited)
   - Makes existing vulnerability worse - tests sanitization tracking

4. SQL injection in basket lookup (routes/basket.ts:47-48)
   - New function findBasketByOwner() with string concatenation
   - Source: req.params.owner → Sink: sequelize.query()

Expected SAST results: 4 NEW + 6-9 UNCHANGED baseline vulnerabilities

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…zed query

Security Fix:
- Replaced unsafe MongoDB $where operator with parameterized field matching
- Previous: db.ordersCollection.find({ $where: `this.orderId === '${id}'` })
- Fixed: db.ordersCollection.find({ orderId: id })

This eliminates arbitrary JavaScript execution vulnerability (CWE-943).

Baseline vulnerability at routes/trackOrder.ts:18 is now remediated.
Expected SAST classification: FIXED

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Added route registration for searchOrders() function:
- Route: GET /rest/orders/search
- Vulnerable to NoSQL injection via $where operator
- Source: req.query.email → Sink: ordersCollection.find()

This makes the vulnerability exploitable with complete data flow.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants