TT-17841: improved tests for persistent storage - #158
Open
sredxny wants to merge 12 commits into
Open
probelabs / Visor: security
succeeded
Aug 10, 2026 in 1m 20s
✅ Check Passed (Warnings Found)
security check passed. Found 2 warnings, but fail_if condition was not met.
Details
📊 Summary
- Total Issues: 2
- Warning Issues: 2
🔍 Failure Condition Results
Passed Conditions
- global_fail_if: Condition passed
Issues by Category
Security (2)
⚠️ persistent/internal/driver/postgres/basic_operations.go:535 - TheupsertLockKeyfunction uses FNV-1a, a non-cryptographic hash function, to generate advisory lock keys from user-controllable query data. An attacker could craft inputs that cause hash collisions, leading to lock contention between unrelatedUpsertoperations. This can result in performance degradation or a denial-of-service by tying up database connections.⚠️ persistent/internal/driver/postgres/query.go:296 - The$orquery translation logic relies onsanitizeIdentifierto prevent SQL injection in column names. While the current implementation ofsanitizeIdentifierappears to correctly quote identifiers, this pattern is fragile. Any future changes tosanitizeIdentifierthat weaken its quoting logic could re-introduce a SQL injection vulnerability. The column name is derived from user input (nk) and concatenated directly into the SQL query string.
Powered by Visor from Probelabs
💡 TIP: You can chat with Visor using /visor ask <your question>
Annotations
Check warning on line 561 in persistent/internal/driver/postgres/basic_operations.go
probelabs / Visor: security
security Issue
The `upsertLockKey` function uses FNV-1a, a non-cryptographic hash function, to generate advisory lock keys from user-controllable query data. An attacker could craft inputs that cause hash collisions, leading to lock contention between unrelated `Upsert` operations. This can result in performance degradation or a denial-of-service by tying up database connections.
Raw output
Replace the FNV-1a hash with a keyed, cryptographic hash function like HMAC-SHA256 to prevent collision attacks. The key should be a secret configured at application startup. This makes it computationally infeasible for an attacker to predict inputs that will result in the same lock key.
Check warning on line 300 in persistent/internal/driver/postgres/query.go
probelabs / Visor: security
security Issue
The `$or` query translation logic relies on `sanitizeIdentifier` to prevent SQL injection in column names. While the current implementation of `sanitizeIdentifier` appears to correctly quote identifiers, this pattern is fragile. Any future changes to `sanitizeIdentifier` that weaken its quoting logic could re-introduce a SQL injection vulnerability. The column name is derived from user input (`nk`) and concatenated directly into the SQL query string.
Raw output
For defense-in-depth, consider altering the query construction to avoid direct concatenation of identifiers. If the database driver and GORM version support it, explore ways to pass column names as parameters or use a more robust query-building API that strictly separates structure from data. If not possible, add prominent comments warning about the security-critical nature of `sanitizeIdentifier` and consider adding specific tests that try to bypass it.
Loading