DEV Community

Cover image for The Secure Code Review Challenge β€” Solution #3: Dice 🎲 (When Input Sanitization Is Not Enough)
Mohamed AboElKheir
Mohamed AboElKheir

Posted on

The Secure Code Review Challenge β€” Solution #3: Dice 🎲 (When Input Sanitization Is Not Enough)

πŸ“’ The solution to Challenge #3: Dice is live. Watch the video walkthrough here, or read the full write-up on GitHub.

The Secure Code Review Challenge is a free biweekly series of full, realistic applications with vulnerabilities based on real-world CVEs and writeups β€” you review, identify, and exploit them the way you would in a real security review, not just spot-the-bug pattern recognition.

If you haven't attempted the challenge yet, this is your cue to stop reading, clone the repo, and try it yourself first. Everything below assumes you've already had a go at it β€” no shame either way, but the exercise is worth more if you struggle with it a bit before seeing the answer.

Two quick announcements before we get into it:

  • Challenge #4 is already live in the repo under challenges/ here. The solution to it will follow on September 3rd.
  • The repo uses GitHub Releases for every new challenge and solution drop. If you go to Watch β†’ Custom β†’ Releases on the repo, you'll get notified automatically instead of having to check back manually.

With that out of the way, let's walk through Dice the same way I did in the video β€” following the same seven-step methodology laid out in the repo, end to end.

A Quick Reminder of What We're Reviewing

Dice is a tiny single-page utility β€” no accounts, no database, no sessions. You can roll one or several six-sided dice, and you can submit a comma-separated list of words and get one picked at random, with the full list echoed back. Every request is public and self-contained. That simplicity is exactly what makes this one interesting: with no authentication or authorization model to break, almost the entire review weight shifts to how the app handles the one piece of untrusted text it actually touches β€” the word list.

Part I β€” Building the Mental Model

1. πŸ—ΊοΈ Application Scope & Architecture

As always, the first move is just spinning the app up and playing with it:

docker compose up
Enter fullscreen mode Exit fullscreen mode

Roll a die, roll a couple, submit a word list, get one picked at random. No login screen, nothing gated β€” a genuinely public, low-stakes little app. That already tells you something important for the review: business-logic checks like auth, IDOR, and CSRF are probably not where this challenge lives.

Reading the stack next:

  • Node.js + Express 5 for the backend β€” one file, index.js, holds every route
  • DOMPurify (backed by jsdom, since DOMPurify needs a DOM to run server-side) β€” an HTML sanitizer
  • unorm β€” a Unicode normalization library
  • A static HTML/JS front end served by Express, with no templating engine

The docker-compose.yml builds the local Dockerfile (node:20-alpine) and publishes port 3000 β€” one container, nothing else running alongside it.

index.js is where the Express backend app lives and has multiple routes. The front end matters just as much here as the backend. views/index.html calls the API with fetch and writes the JSON response straight back into the page. It also auto-runs the word selector from a ?words= URL parameter on page load.

2. πŸšͺ Entry Points

Since there's no auth anywhere, every entry point is reachable by anyone:

  • GET / β€” untrusted input: ?words= (consumed by client JS on load)
  • GET /api/roll-dice β€” no untrusted input
  • POST /api/roll-dices β€” untrusted input: count (JSON body)
  • GET /api/random-word β€” untrusted input: words (query string)
  • POST /api/random-word β€” untrusted input: words[] (JSON body)

The web page itself is an additional surface worth calling out separately: it takes the same word input from a textarea or from that ?words= URL parameter, and renders whatever the API sends back into the DOM.

3. 🎯 Dangerous Sinks

With business logic largely off the table, the review is really about tracing what happens to the words input. Two things stand out immediately once you open views/index.html:

The page is full of innerHTML assignments β€” the pattern we've flagged as dangerous in earlier challenges in this series, because anything written into innerHTML gets parsed as HTML, not displayed as text. The specific one that matters here is where the API's response gets rendered:

let html = `<div class="selected-word">Selected: ${data.selectedWord}</div>`;   // :230
data.allWords.forEach((word, index) => {
    html += `<div class="word-item">${index + 1}. ${word}</div>`;                // :233
});
resultDiv.innerHTML = html;                                                       // :237
Enter fullscreen mode Exit fullscreen mode

If data.selectedWord or any word contains live HTML, that HTML gets parsed and executed by the browser. The only thing standing between user input and this sink is whatever the server does inside processWords() before returning the words β€” so the whole question of this review becomes: can a payload survive processWords() and still contain live HTML on the other side?

There's also a second, much smaller thing worth flagging while we're in the code, even though it isn't the main event: roll-dices takes a count from the request body with no upper bound:

const count = parseInt(req.body.count) || 1;
Enter fullscreen mode Exit fullscreen mode

The UI caps this at 10, but the API itself doesn't β€” send {"count": 100000000} directly and the server will happily try to build an array that size. It's a minor denial-of-service issue, not the planted vulnerability, but it's a legitimate finding and a good example of why you check the API directly instead of only trusting what the UI lets you send.

4 & 5. 🧩 Threat Modeling and πŸ” Mitigation Review

πŸ”“ Business logic first. This app is intentionally public β€” no accounts, no sessions, no per-object data, nothing state-changing on the server. There's no privilege boundary to cross, so authentication, authorization, IDOR, and CSRF are all not applicable by design. That's a real, quick verdict here, not a corner being cut β€” the app genuinely has nothing in this category to check.

πŸ’‰ Source-to-sink next β€” is the innerHTML sink actually reachable?

Here's where the review earns its keep. Open processWords():

function processWords(words) {
    return words.map(word => {
        let sanitized = DOMPurify.sanitize(word, { ALLOWED_TAGS: [] });   // 1) strip HTML
        sanitized = unorm.nfkc(sanitized);                               // 2) THEN normalize
        return sanitized;
    });
    // ...
}
Enter fullscreen mode Exit fullscreen mode

The obvious first check is: does DOMPurify actually work? It does. Feed it a real <script>alert(1)</script> tag and it comes back empty β€” DOMPurify strips it exactly as expected. On the surface, this looks like a textbook case of "dangerous sink, but properly mitigated." If that were the whole story, this review would be finished: sanitize before render, verified, move on.

But there's a second line in that function β€” unorm.nfkc(sanitized) β€” and it runs after sanitization, not before. That ordering is worth pausing on, because it's a pattern this series has seen before. In Challenge #1, a check ran, and then a modification ran on the already-checked value β€” trimming whitespace after an authorization check had already passed, which let an attacker slip past the check with a value that only became "clean" afterward. This is the same shape of bug: verify first, modify second. Whenever you see that order, the question to ask is: is there an input that passes the check as garbage, but the modification turns into something dangerous?

For a sanitizer specifically, that means asking: is there a character the sanitizer lets through as harmless text, that normalization would then turn into real HTML syntax?

Unicode normalization exists for good reasons β€” folding "strange-looking" Unicode characters into their standard equivalents, useful for search, string comparisons, or making foreign-language input consistent. Run it on stylized text like 𝕒𝕑𝕑𝕝𝕖, π•“π•’π•Ÿπ•’π•Ÿπ•’, 𝕔𝕙𝕖𝕣𝕣π•ͺ and it comes back as plain apple, banana, cherry.

$ node
Welcome to Node.js v24.16.0.
Type ".help" for more information.
> const unorm = require('unorm');
undefined
> s='𝕒𝕑𝕑𝕝𝕖, π•“π•’π•Ÿπ•’π•Ÿπ•’, 𝕔𝕙𝕖𝕣𝕣π•ͺ'
'𝕒𝕑𝕑𝕝𝕖, π•“π•’π•Ÿπ•’π•Ÿπ•’, 𝕔𝕙𝕖𝕣𝕣π•ͺ'
>  unorm.nfkc(s);
'apple, banana, cherry'
>
Enter fullscreen mode Exit fullscreen mode

That's the intended use β€” but the same folding applies to a much wider set of "compatibility" characters than most people realize, including two that matter a great deal here: the fullwidth less-than (<, U+FF1C) and fullwidth greater-than (>, U+FF1E) characters. Visually, these are almost indistinguishable from ordinary < and > β€” genuinely hard to spot just by eye. NFKC normalizes both straight to their ASCII equivalents.

That's the bypass. < and > are not HTML syntax, so DOMPurify treats them as ordinary text and lets them straight through. The unorm.nfkc() call that follows then rewrites them into real < and > β€” reconstructing a live HTML tag after the sanitizer already had its say and considered the string safe.

Part II β€” Finding, Exploiting, and Fixing the Bug

6. πŸ§ͺ The Vulnerability: Sanitizer Bypass Through the Wrong Order of Operations

You can see the bypass directly by running processWords's two steps side by side on two different inputs:

INPUT       : "<img src=x onerror=alert(1)>"      (real ASCII tag)
afterPurify : ""                                   ← DOMPurify strips it βœ”
afterNFKC   : ""

INPUT       : "<img src=x onerror=alert(1)>"      (fullwidth tag)
afterPurify : "<img src=x onerror=alert(1)>"      ← passes through as "text"
afterNFKC   : "<img src=x onerror=alert(1)>"       ← normalization revives the tag πŸ’₯
Enter fullscreen mode Exit fullscreen mode

One quick note on payload choice: a fullwidth <script> tag normalizes just as well, but browsers don't execute <script> elements that get inserted via innerHTML β€” that's a deliberate HTML5 security behavior. For an innerHTML sink, the reliable primitive is an event-handler-bearing tag like <img src=x onerror=...> or <svg onload=...>, which is why the PoC uses onerror.

Proving it against the running server β€” both endpoints reconstruct a live tag from the fullwidth payload:

# POST (JSON body). --data-binary preserves the multibyte UTF-8 characters.
curl -s -X POST http://localhost:3000/api/random-word \
  -H "Content-Type: application/json" \
  --data-binary '{"words": ["<img src=x onerror=alert(document.domain)>"]}'
# β†’ {"selectedWord":"<img src=x onerror=alert(document.domain)>",
#     "allWords":["<img src=x onerror=alert(document.domain)>"],"originalCount":1}
Enter fullscreen mode Exit fullscreen mode

The response contains a real <img onerror=...> tag β€” the sanitizer has been bypassed server-side, before the payload ever reaches the browser.

And because the front end auto-runs the word selector from ?words= on page load, this isn't just a curl trick β€” it's a complete, one-click XSS delivered as a single link:

http://localhost:3000/?words=%EF%BC%9Cimg%20src%3Dx%20onerror%3Dalert(document.domain)%EF%BC%9E
Enter fullscreen mode Exit fullscreen mode

(%EF%BC%9C / %EF%BC%9E are just the URL-encoded UTF-8 bytes for < / >.) A victim opens that link, the page fetches the word selector automatically, the server hands back a live <img> tag, the UI drops it into innerHTML, the image fails to load, and onerror fires β€” arbitrary JavaScript running in the victim's origin, with no clicks beyond opening the link.

XSS proof-of-concept β€” the injected onerror handler fires an alert in the victim's browser

There's no session or cookie to steal in this particular toy app, but in a real application this is the standard DOM XSS blast radius: session hijack, keylogging, forged same-origin requests, phishing overlays laid over the real page β€” all triggered by a link that looks completely unremarkable.

7. πŸ› οΈ The Fix

The fix is genuinely one line β€” swap the order:

const sanitizedWords = words.map(word => {
    const normalized = unorm.nfkc(word);                          // 1) canonicalize FIRST
    return DOMPurify.sanitize(normalized, { ALLOWED_TAGS: [] });   // 2) THEN sanitize (last transform)
});
Enter fullscreen mode Exit fullscreen mode

With normalization running first, <img β€¦οΌž becomes <img …> before DOMPurify ever sees it β€” so DOMPurify strips it like any other tag, and the bypass is closed.

That one-line reorder is the core fix, but as with every challenge in this series, it shouldn't be the only control. A few things worth layering on top:

  • Fix the sink, not just the source. innerHTML is still doing string concatenation of untrusted data β€” that's the underlying risk, independent of this specific bug. Building the DOM with textContent / createElement instead would have neutralized this payload even with the ordering bug still in place:
const sel = document.createElement('div');
sel.className = 'selected-word';
sel.textContent = `Selected: ${data.selectedWord}`;
resultDiv.replaceChildren(sel);
// build each word row with createElement + textContent too
Enter fullscreen mode Exit fullscreen mode
  • Add a Content-Security-Policy. A restrictive CSP (default-src 'self'; script-src 'self'; object-src 'none') would block inline onerror= handlers from executing at all β€” a second independent layer catching the same class of bug. Note the app currently uses inline on* handlers elsewhere, so adopting a strict CSP means moving that JS out of attributes first.
  • Input validation / allow-listing on the word fields β€” a length cap and a character allow-list makes it much harder to smuggle any kind of markup through in the first place.
  • Clamp count on POST /api/roll-dices (e.g. Math.min(Math.max(1, count), 10)) to close the unbounded-loop DoS finding from earlier.

8. Why This Matters Beyond unorm and DOMPurify

The individual pieces here are each, on their own, correct. DOMPurify does exactly what it promises β€” it strips real HTML tags. unorm.nfkc() does exactly what it promises β€” it folds compatibility Unicode characters to their canonical form. Neither library has a bug. The vulnerability lives entirely in the order they were wired together in.

This is the exact same underlying pattern as Challenge #1's authorization bypass, just wearing different clothes: a check ran, and then a transformation ran on the value after the check had already passed judgment on it. The check itself was never the weak point in either case β€” the sequencing was. This has a name: CWE-180 β€” Incorrect Behavior Order: Validate Before Canonicalize, a close cousin of CWE-179, and it's exactly what DOMPurify's own documentation warns against β€” treat its output as final, and never transform it afterward.

The broader takeaway for how you review code: whenever a pipeline chains multiple transforms β€” decode, normalize, trim, canonicalize, validate, sanitize β€” the order matters as much as the presence of each individual step. A sanitizer that runs before a canonicalizing transform isn't sanitizing the value that actually gets used; it's sanitizing an intermediate value that never sees the light of day. The rule to carry forward: canonicalize/normalize/decode first, validate/sanitize last β€” and once sanitization has run, that value should be the final one that reaches the sink, full stop.

Wrapping Up

If you worked through Dice yourself, I'd like to know: did you spot the ordering issue on your own, or did DOMPurify's "clean" output on a normal payload convince you to move on before you looked at what came after it?

Challenge #4 is live now if you're ready for the next one, and I'll be back on September 3rd with its solution and a new challenge alongside it.

Links:

Top comments (0)