Insecure Randomness

[ahpaleus]

Severity: High

The caddy-security plugin uses the math/rand Golang library with a seed based on the Unix timestamp to generate strings for three security-critical contexts in the application, which could possibly be predicted via a brute-force search. Attackers could use the potentially predictable nonce value used for authentication purposes in the OAuth flow to conduct OAuth replay attacks. In addition, insecure randomness is used while generating multifactor authentication (MFA) secrets and creating API keys in the database package.

• https://github.com/greenpau/go-authcrunch/blob/a6e434247876ad0b01940ac2947f17ca32a714bf/pkg/idp/oauth/authenticate.go#L204-L204
• https://github.com/greenpau/go-authcrunch/blob/a6e434247876ad0b01940ac2947f17ca32a714bf/pkg/authn/handle_http_login.go#L193-L193

To immediately mitigate this vulnerability, use a cryptographically secure random number generator for generating the random strings. Golang’s library crypto/rand is designed for secure random number generation.

In addition to that fix, we recommend considering the following long-term recommendations:

• Review the application for other instances where the math/rand package is used for secure context. Create secure wrapping functions and use them throughout the code to serve a cryptographically secure string with the requested length.
• Avoid duplicating code. Having a single function, such as secureRandomString, rather than multiple duplicate functions makes it easier to audit and verify the system’s security. It also prevents future changes to the codebase from reintroducing issues.
• Implement Semgrep in the CI/CD. The math-random-used Semgrep rule will catch instances where math/rand is used. Refer to our Testing Handbook on Semgrep for more information.
• Read textbooks such as Real World Cryptography, as it is a great resource for practical cryptographic considerations.

More information about our public disclosure:

• https://blog.trailofbits.com/2023/09/18/security-flaws-in-an-sso-plugin-for-caddy/

[st3fan]

@greenpau let me know if you accept contributions for this issue. I was reading the util code in go-authcrunch and I think it is relatively simple to convert it to use crypto/rand instead. I'm happy to give that a shot.

[greenpau]

@st3fan , please do! 😄 thank you for the offer!

[gedw99]

Thos looks like a simple fix. Is anyone working on it ?

[greenpau]

@gedw99 , feel free to contribute!

[greenpau]

@ahpaleus , please see if the fix addresses your concerns.

[greenpau]

No activity. Closing.