Add Vietnamese SQL vulnerability notes and examples - #2
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05832b68b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const where = columns.map((column) => `${column} LIKE ?`).join(' OR '); | ||
|
|
||
| const rows = await db.query( | ||
| `SELECT id, name, sku FROM products WHERE ${where} LIMIT 50`, |
There was a problem hiding this comment.
When every requested column is absent from SEARCHABLE_COLUMNS (or the client sends only invalid column names), columns becomes empty and this builds SELECT ... WHERE LIMIT 50, so the “safe” example fails with invalid SQL instead of rejecting the request or falling back to a valid predicate. Because this snippet is presented as the safer pattern, it should explicitly handle the zero-column case before constructing the query.
Useful? React with 👍 / 👎.
|
|
||
| ```js | ||
| app.get('/orders', async (req, res) => { | ||
| const limit = Math.min(Number(req.query.limit) || 50, 100); |
There was a problem hiding this comment.
For requests such as ?limit=-1, Number(req.query.limit) || 50 evaluates to -1, and Math.min(-1, 100) keeps the negative value, so the supposedly safe pagination example can still emit an invalid/driver-dependent LIMIT ?. Since the text recommends this as the safe version, clamp the lower bound as well as the upper bound before passing it into SQL.
Useful? React with 👍 / 👎.
| await db.beginTransaction(); | ||
| for (const item of items) { | ||
| await db.query( | ||
| 'UPDATE products SET stock = stock - ? WHERE id = ?', | ||
| [item.quantity, item.product_id] | ||
| ); | ||
| } | ||
| await db.commit(); |
There was a problem hiding this comment.
If any UPDATE rejects after beginTransaction(), this safer deadlock/lock-contention example skips commit() but never calls rollback(), leaving the connection in an open transaction that can hold locks until the pool discards or resets it. Because the section is teaching how to keep transactions short, wrap the transaction body in try/catch and roll back on failure.
Useful? React with 👍 / 👎.
1 participant
Motivation
Description
README.mdwith a Vietnamese summary and a linked index of vulnerability topics in thevulnerabilities/directory.vulnerabilities/sql-injection.md,vulnerabilities/blind-sql-injection.md,vulnerabilities/second-order-sql-injection.md,vulnerabilities/excessive-query-complexity.md,vulnerabilities/unbounded-result-sets.md,vulnerabilities/lock-contention-deadlocks.md, andvulnerabilities/missing-rate-limits.md.Testing
Codex Task