fix: the basicauth middleware accepts sha-256 hex-en... in config.go#4295
fix: the basicauth middleware accepts sha-256 hex-en... in config.go#4295orbisai0security wants to merge 1 commit into
Conversation
Automated security fix generated by OrbisAI Security
|
Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR tightens BasicAuth password hash validation by removing hex-decoding fallback for SHA-256 hashes and adding explicit error reporting for unsupported formats. The import change and error handling work together to restrict support to bcrypt, SHA512, and SHA256 formats only. ChangesBasicAuth Hash Format Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the basicauth middleware to remove the default SHA256 password hashing fallback, returning an error for unsupported formats instead. Feedback suggests improving the error message for consistency and security by recommending bcrypt exclusively and removing the now-unused ErrInvalidSHA256PasswordLength error variable.
| sum := sha256.Sum256([]byte(p)) | ||
| return subtle.ConstantTimeCompare(sum[:], b) == 1 | ||
| }, nil | ||
| return nil, fmt.Errorf("basicauth: unsupported password hash format; use bcrypt ($2…), {SHA512}, or {SHA256}") |
There was a problem hiding this comment.
The error message is inconsistent with other errors in this function (which do not use the basicauth: prefix) and uses a Unicode ellipsis (…) instead of standard ASCII. Additionally, since the PR aims to address security concerns with SHA-256, it is better to recommend bcrypt exclusively in the error message, as {SHA256} and {SHA512} are also unsalted and fast hashes that are not recommended for secure password storage. Also, ErrInvalidSHA256PasswordLength is now unused and should be removed.
| return nil, fmt.Errorf("basicauth: unsupported password hash format; use bcrypt ($2…), {SHA512}, or {SHA256}") | |
| return nil, errors.New("unsupported password hash format; use bcrypt ($2...) for secure password storage") |
|
@ReneWerner87 Can we ban this account. This is the 2nd AI slop PR it submits. Probably a bot. Rel: #4257 |
3 participants
Summary
Fix high severity security issue in
middleware/basicauth/config.go.Vulnerability
V-001middleware/basicauth/config.go:17Description: The BasicAuth middleware accepts SHA-256 hex-encoded password hashes as a valid credential format when no algorithm prefix is supplied. SHA-256 is a general-purpose cryptographic hash function, not a password hashing algorithm. It lacks per-user salting, making it trivially vulnerable to precomputed rainbow table attacks. It is also orders of magnitude faster to compute than bcrypt, enabling GPU-accelerated brute-force attacks that can test billions of candidates per second. An attacker who obtains the stored hash through a database breach or configuration file exposure can recover the plaintext password far more quickly than with bcrypt.
Changes
middleware/basicauth/config.goVerification
Automated security fix by OrbisAI Security