Skip to content

demo(09-insecure-random-token): 09 — System.Random used to mint a confirmation token#158

Open
EricCogen wants to merge 1 commit intomainfrom
demo/09-insecure-random-token
Open

demo(09-insecure-random-token): 09 — System.Random used to mint a confirmation token#158
EricCogen wants to merge 1 commit intomainfrom
demo/09-insecure-random-token

Conversation

@EricCogen
Copy link
Copy Markdown
Owner

09 — System.Random used to mint a confirmation token

Expected verdict: ❌ Fails — GauntletCI should fire GCI0048 (insecure random in security context).

What changed

A new helper generates the per-order email confirmation token using
System.Random:

public sealed class EmailConfirmationTokenGenerator
{
    public string NewConfirmationToken()
    {
        var rng = new Random();
        var token = new char[16];
        for (var i = 0; i < token.Length; i++)
        {
            token[i] = (char)('a' + rng.Next(26));
        }
        return new string(token);
    }
}

System.Random is a non-cryptographic PRNG. Its output is fully
predictable from its seed, and from observing a handful of values an
attacker can recover the internal state and predict future tokens.

Why this is risky

  • A predictable confirmation token lets an attacker complete the
    confirm-email flow for any pending order without ever receiving the
    email.
  • Even ignoring seeding attacks, two new Random() instances created
    in the same tick share a seed on older runtimes and emit identical
    sequences — perfect for collision attacks against shared inboxes.
  • RandomNumberGenerator.GetBytes / GetHexString is one line of code
    and is the only correct primitive here.

What GauntletCI catches

GCI0048 Insecure Random in Security Contextnew Random( appears
within five lines of an identifier that names a security-sensitive
value (here, token).

…firmation token

See scenarios/09-insecure-random-token/README.md for the expected verdict.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GauntletCI found issues in this PR. See inline comments for details.

{
public string NewConfirmationToken()
{
var rng = new Random();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCI0048 — Insecure Random in Security Context

System.Random used near security-sensitive identifier — use a cryptographic RNG instead

Evidence:

Line 7: var rng = new Random();

⚠️ Why it matters: System.Random is a pseudo-random number generator seeded from the system clock. Its output is predictable and must never be used for cryptographic purposes such as generating tokens, keys, salts, nonces, or passwords.

💡 Suggested action: Replace with RandomNumberGenerator.GetBytes() or RandomNumberGenerator.GetHexString() (.NET 8+) for security-sensitive values.

Confidence: High | Severity: Info

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d062a4d125

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

{
public string NewConfirmationToken()
{
var rng = new Random();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use a cryptographic RNG for confirmation tokens

NewConfirmationToken mints a security token with System.Random, which is predictable and can repeat across closely timed calls, so an attacker can guess or collide confirmation values and confirm orders they do not control. Because this value gates an account/order action, it should be generated with a cryptographically secure source (for example RandomNumberGenerator) instead of a non-crypto PRNG.

Useful? React with 👍 / 👎.

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