-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Security Assessment Report: tinh-tinh/sqlorm
Overall Risk Level: HIGH
Target: github.com/tinh-tinh/sqlorm
Version Analyzed: c5bbc7f5f428431b37051cae6c2723c865a62291
1. Executive Summary
While the project demonstrates strong commitment to security hygiene and DevSecOps practices, the tenancy (multi-tenancy) module contains critical SQL Injection vulnerabilities and architectural flaws that could lead to Denial of Service (DoS). The repository effectively utilizes automated scanning tools, but the manual implementation of dynamic queries exposes the system to severe risks.
2. Security Controls & Hygiene (Strengths)
The project implements several industry-standard security practices in its development workflow:
- Automated Vulnerability Scanning (SAST): The repository integrates CodeQL via GitHub Actions to perform semantic code analysis on every push and pull request.
- Dependency Auditing (SCA): The
Makefileincludes anaudittarget that runsgovulncheck, identifying known vulnerabilities in project dependencies. - Strict Linting: A
pre-commithook enforcesgolangci-lintto catch basic errors and anti-patterns before code is committed. - ORM Usage: Leveraging GORM as the foundation helps mitigate SQL Injection risks for standard CRUD operations when used correctly.
3. Critical Findings
3.1. SQL Injection in Multi-tenancy Module (Critical)
Description: The CreateDatabaseIfNotExist function in tenancy/tenancy.go constructs SQL statements by directly formatting strings with user input using fmt.Sprintf.
Location: tenancy/tenancy.go
Analysis:
- The code executes statements like
stmt := fmt.Sprintf("SELECT * FROM pg_database WHERE datname = '%s';", dbName)andstmt := fmt.Sprintf("CREATE DATABASE %s;", dbName). - The
dbNamevariable (derived fromtenantID) is taken directly from the request (e.g., thex-tenant-idheader in tests) without validation. - Exploitation: An attacker can send a malicious header such as
x-tenant-id: test'; DROP TABLE pg_database; --to execute arbitrary SQL commands with high privileges (as database creation requires administrative rights).
3.2. SQL Injection in Query Builder (High)
Description: The functions in builder.go concatenate column names directly into query strings.
Location: builder.go (Functions: Equal, In, MoreThan, Like, etc.)
Analysis:
- Example code:
query := column + " = ?". - While GORM protects the value using parameterization (
?), it does not protect the column name. If the application allows users to sort or filter by arbitrary fields (e.g.,?filter_by=name'--), attackers can inject SQL into the column name segment.
3.3. Denial of Service (DoS) via Tenant Connection Management (High)
Description: The ForRoot mechanism in tenancy.go caches database connections in a global ConnectMapper map without any size limit or eviction policy.
Location: tenancy/tenancy.go
Analysis:
- The code checks
if mapper[tenantID] == niland, if true, creates a new connection and permanently stores it in the map. - Exploitation: An attacker can flood the server with thousands of requests containing unique
x-tenant-idvalues. The application will attempt to open thousands of database connections (exhausting the connection pool) and bloat the memory usage of theConnectMappermap indefinitely, leading to a crash.
4. Other Risks
4.1. Insecure SSL Configuration (Medium)
- Description: The database connection string in
tenancy/tenancy.gohardcodes the settingsslmode=disable. - Impact: Sensitive data transmitted between the application and the database is not encrypted, making it vulnerable to Man-in-the-Middle (MitM) attacks.
4.2. Lack of Tenant Authorization (IDOR Risk)
- Description: The
tenantIDis trusted explicitly from the request header. The provided code does not verify if the current user is authorized to access the requested tenant. If a user discovers another client'stenant-id, they can potentially access that client's data.
5. Recommendations
-
Remediate SQL Injection Immediately:
- In
tenancy.go: Never use string concatenation for identifiers. Use the SQL driver's identifier quoting mechanisms or strictly whitelist allowed characters (e.g., alphanumeric only) for database names. - In
builder.go: Validatecolumnnames against a strict allowlist of struct fields before using them in queries.
- In
-
Resource Management for Multi-tenancy:
- Implement an LRU Cache for
ConnectMapperto limit the maximum number of active connections held in memory. - Implement Rate Limiting on the creation of new tenant connections.
- Implement an LRU Cache for
-
Security Configuration:
- Expose
sslmodeas a configurable option inConnectOptionsrather than hardcoding it todisable. - Add documentation or middleware warnings regarding the use of
x-tenant-idwithout proper authorization checks.
- Expose
Sub-issues
Metadata
Metadata
Assignees
Labels
Type
Projects
Status