Skip to content

fix: replace hardcoded JWT secret and DB password with env vars (CWE-798)#161

Open
saaa99999999 wants to merge 1 commit into
eddycjy:masterfrom
saaa99999999:fix/hardcoded-credentials
Open

fix: replace hardcoded JWT secret and DB password with env vars (CWE-798)#161
saaa99999999 wants to merge 1 commit into
eddycjy:masterfrom
saaa99999999:fix/hardcoded-credentials

Conversation

@saaa99999999

Copy link
Copy Markdown

Summary

go-gin-example 的 conf/app.ini 中硬编码了两个关键安全凭据:

  • JwtSecret = 233 → JWT 签名密钥弱且硬编码,可被攻击者从 GitHub 获取后伪造任意用户 token
  • Password = rootroot → 数据库密码硬编码,泄露后可直接访问数据库

Changes

pkg/setting/setting.go — 添加环境变量覆盖逻辑,密钥为空时 fatal exit:

if jwtSecret := os.Getenv("JWT_SECRET"); jwtSecret != "" {
AppSetting.JwtSecret = jwtSecret
}
if AppSetting.JwtSecret == "" {
fmt.Fprintf(os.Stderr, "fatal: JWT_SECRET is not set...")
os.Exit(1)
}

conf/app.ini — 移除硬编码值,置空由环境变量提供:
-JwtSecret = 233
+JwtSecret =
-Password = rootroot
+Password =

Data Flow

conf/app.ini:JwtSecret → setting.AppSetting.JwtSecret → util.jwtSecret → jwt.SignedString()/ParseToken()
conf/app.ini:Password → setting.DatabaseSetting.Password → models.Setup() → gorm.Open(DSN)

Verification

启动前设置环境变量:

export JWT_SECRET="$(openssl rand -base64 32)"
export DB_PASSWORD="your-db-password"

…798)

JwtSecret=233 and Password=rootroot in conf/app.ini replaced with
JWT_SECRET/DB_PASSWORD environment variables with fatal-on-empty guard.
@saaa99999999

Copy link
Copy Markdown
Author

Hi maintainer, thank you for reviewing this PR. This fix addresses CWE-798 (Use of Hard-coded Credentials) with two findings:

  1. JwtSecret = 233 in conf/app.ini — attacker can obtain this from GitHub and forge arbitrary user tokens
  2. Password = rootroot in conf/app.ini — database credential exposure

Could you please apply for a CVE ID from GitHub Advisory (Security tab > Draft a security advisory) for these vulnerabilities after merging? This is the correct process: the CVE should be requested on the official upstream repository (eddycjy/go-gin-example), not on the fork. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant