DEV Community

Cover image for Extracting a QR login from a production app and closing 11 security holes before merge
Dmitry Isaenko
Dmitry Isaenko

Posted on

Extracting a QR login from a production app and closing 11 security holes before merge

QR login is one of those features that looks tiny on the roadmap and turns out to be a security minefield. You show a QR code in the browser, scan it with a phone you are already signed in on, and the web session logs itself in. WhatsApp Web, Telegram Web, Steam. Everyone has used it. Nobody thinks about what happens behind it.

I had a working version of this in an app I already run in production. So when it came time to add cross-device login to the LaraFoundry core, I did what I do with every module: I extracted the real code instead of writing a fresh one from a blog post. Extracting beats greenfield because the logic already survived contact with real users. But there is a catch, and it is the whole point of this post: code that works is not the same as code that is safe. The donor version worked fine and had eleven things I was not willing to ship.

This is the list. Every hole, and what it became.

The flow, in one paragraph

The browser (a guest, not logged in) asks the backend for a sign-in request and renders it as a QR. A second device that is already authenticated scans the code and hits a verify endpoint to approve it. Meanwhile the browser polls until the request flips to approved, then logs the user in. Three endpoints: generate, verify, poll. Simple shape, lots of sharp edges.

The eleven

# The donor did this The core does this
1 Stored the raw token in the DB and put it in the QR URL Stores a SHA-256 hash; the plaintext lives only in the QR image
2 No rate limit on any endpoint Throttle on generate, poll and verify (verify is the brute-force surface)
3 axios.get(decodedText) on whatever the QR decoded to Validates the scanned URL (same origin plus the exact verify path) before any request
4 TTL slid forward forever on reuse Absolute cap measured from creation, so a code dies even if it keeps refreshing
5 Expired rows piled up forever A prune command the host schedules
6 Auth::login() then session()->regenerate() Regenerate first, then log in (session fixation)
7 No audit of attempts Every approve, fail and block goes to the activity log
8 No indexes on the columns it queried Indexes on the poll and prune predicates
9 Decrypted the id from the URL, so a bad value threw a 500 No decryption at all; a bad code is a clean 404
10 Encrypted the token AND the id into the session, twice Stores only the row id, once
11 An unexplained subSeconds(13) fudge on the expiry check Gone; a plain expires_at > now()

Three of these are worth showing in code, because they are the ones people get wrong everywhere, not just here.

Hole 1: the token was plaintext in the database

The donor generated a UUID, stored it as-is, and embedded it in the QR URL. If your database leaks, every live QR code leaks with it. The token is a bearer secret, so it gets the same treatment as a password: hash it, compare hashes, never persist the original.

// generate(): the plaintext only ever lives in the QR, the DB gets a hash
$plain = (string) Str::uuid();

$signInRequest = SignInRequest::create([
    'token'      => hash('sha256', $plain),
    'expires_at' => now()->addMinutes($this->ttlMinutes()),
    'ip_address' => $request->ip(),
    'user_agent' => $request->userAgent(),
]);

// verify(): look the row up by the hash of the token from the URL
$signInRequest = SignInRequest::query()
    ->where('id', $id)
    ->where('token', hash('sha256', $token))
    ->where('approved', false)
    ->where('expires_at', '>', now())
    ->first();
Enter fullscreen mode Exit fullscreen mode

A nice side effect of dropping the encrypt/decrypt dance: there is nothing left to throw. The donor wrapped the id in Crypt::encrypt(), so a malformed value blew up with a 500 (hole 9). Hashing is a string compare. A wrong code just does not match, and you return a clean 404.

Hole 3: the scanner fetched whatever it decoded

This one made me wince. The mobile side did this:

// the donor: fetch literally anything the camera decoded
const onScanSuccess = async (decodedText) => {
    await axios.get(decodedText)
    location.reload()
}
Enter fullscreen mode Exit fullscreen mode

A QR code is attacker-controlled input. Point the camera at a malicious code and the page will happily issue a request to any URL it contains. That is a server-side request forgery and open-redirect vector handed to you for free. The fix is to refuse to fetch anything that is not our own verify endpoint, same origin, right path, real scheme:

function safeVerifyUrl(decoded) {
    let url
    try {
        url = new URL(decoded, window.location.origin)
    } catch {
        return null
    }
    // reject blob:/data:/javascript: before anything else
    if (url.protocol !== 'https:' && url.protocol !== 'http:') return null
    if (url.origin !== window.location.origin) return null

    return /^(?:\/[^/]+)?\/larafoundry\/qr\/verify\/\d+\/[^/]+$/.test(url.pathname)
        ? url.toString()
        : null
}
Enter fullscreen mode Exit fullscreen mode

The URL parser normalizes encoding and path traversal before the regex ever runs, so the ..%2f tricks do not get through. Origin is checked against the live origin, not a string match, so localhost.evil.com and userinfo tricks like app@evil.com fail too. A reviewer later threw 25 bypass vectors at this and every dangerous one bounced.

Hole 6: log in first, ask questions later

The donor logged the user in and then regenerated the session. That ordering is a session-fixation invitation: an attacker who fixes the session id before login keeps a valid handle after it. Flip the two lines and the fixated id is thrown away before the user is ever attached to it.

// poll(): new session id BEFORE the identity is attached to it
$request->session()->regenerate();
Auth::guard('web')->login($user);

$signInRequest->delete(); // single-use: the approved code cannot be replayed
Enter fullscreen mode Exit fullscreen mode

The delete() is its own small fix. The donor left the approved row sitting there, so the same code could be polled again. One use, then it is gone.

The thing the donor never had: the super-admin block

There is a rule in the platform that the operator account cannot sign in by QR. The donor checked a raw is_admin column. The core already has a single resolver for "is this the platform super-admin", with a case-insensitive email allow-list on top of the flag, so a flipped column alone cannot grant it. The QR verify reuses that one resolver instead of inventing a second check that can drift:

if ($this->visitorStatus->isAdmin($approver)) {
    // audited, then refused
    return response()->json(['error' => __('larafoundry::auth.qr.admin_forbidden')], 403);
}
Enter fullscreen mode Exit fullscreen mode

One source of truth for a security decision beats two that agree today and disagree after a refactor.

While I was in there: page or modal

The same phase shipped a small unrelated quality-of-life thing. The auth screens (login, register, forgot, reset) can now render either as a full page or as a modal over the host's content, switched by one config value:

'presentation' => env('LARAFOUNDRY_AUTH_PRESENTATION', 'page'), // page | modal
Enter fullscreen mode Exit fullscreen mode

Default is page, so nothing changes for anyone who does not opt in. The donor only had modals, the core only had pages. Neither had the switch. Now one config key picks the surface and the same screen component renders both ways. An unknown value falls back to page, so a typo can never produce an undefined surface.

Proof, not vibes

Every module in the core ships with Pest tests, and a security feature gets the heaviest coverage. The QR backend has a test that runs the full two-device handshake inside one process (two independent sessions): a guest generates, an approver verifies, the guest polls and ends up authenticated, the row is consumed. On top of that: the token is stored hashed and never in plaintext, an expired code is refused, the absolute cap holds, a super-admin is blocked with a 403, a bogus token returns a clean 404 and never a 500, the verify works under both a web session and a Bearer token, and the prune command clears the right rows. The full core suite is 497 tests green.

The part I am proudest of is the part that caught me

Here is the build-in-public bit. Every sub-phase of this work went through two adversarial reviewers before merge, whose job is to try to break it, not to praise it. On the QR backend they caught a HIGH that I had missed: the verify URL carries the token, and the activity log was recording the full request URL. So the token I had so carefully hashed in the database was sitting in plaintext in the audit log. The hashing was undone by the logging.

The fix was systemic, not a patch on one line: the activity context now redacts secret path segments (by route-parameter name) the same way it already redacted query-string secrets. One reviewer, one HIGH, one better fix than I would have written if I had shipped on my own confidence. That is the whole reason the review gate exists.

Follow along

This is part of LaraFoundry, a reusable SaaS/CRM core for Laravel that I am extracting in public from real, production code.

The guard-agnostic verify endpoint (one controller that serves both a web session today and a mobile Bearer token tomorrow) deserves its own short write-up. That is the next post in the series.

Top comments (0)