Skip to contentExploitQuest

Lesson 1 of 1 in The Confident Wrong Answer

Plausible, Idiomatic, Broken

The code that gets reviewed least carefully is the code that looks most like code you would have written.

3 min read

Not yet reviewed

Bad code announces itself. It is oddly formatted, it does things in a strange order, and it makes a reviewer slow down — which is the useful part. Generated code is fluent. It is idiomatic for the language, it is named the way you would name it, and it reads as though somebody competent wrote it, because in a statistical sense somebody competent did.

Note

Fluency is not correctness, and it *suppresses* the reflex that catches incorrectness. This is the whole chapter: the risk is not that the code is worse, it is that your review of it is.

Three that read perfectly

Looks right

const user = await db.getUser(req.body.userId)
return user.invoices

What is missing

Nothing checks that the session owns that userId.
The prompt said "return the user's invoices" and this
does exactly that - for whichever user was asked for.

A reviewer skims this and sees a fetch and a return, in the shape those always take. The absent line is invisible because absence always is.

Looks right

if (token === session.csrfToken) { ... }

What is missing

A short-circuiting comparison on a secret. Correct
output, wrong timing, and the version that reads
most naturally is the one that leaks.

Models produce the idiomatic form, and the idiomatic form of comparing two strings is ===. Being idiomatic is what makes this hard to see.

Looks right

const filename = path.join(UPLOADS, req.body.name)
await fs.writeFile(filename, data)

What is missing

`path.join` normalises `..` rather than rejecting it.
The name came from a request, so the path is the
requester's to choose.

This one is worse than the others because path.join *looks like* the safe thing to do. It is the function you reach for precisely when you are being careful about paths.

None of these is exotic. Every one is a bug this platform's other courses teach, in the plainest form, produced in the most idiomatic way the language offers.

Reading for absence

Ordinary review reads what is there. Reviewing generated code means reading for what is not — the authorization check, the constant-time comparison, the rejection of a path that escapes. Absence has no syntax highlighting and no line number.

Who is allowed to do this, and where is that decided?
What happens with the empty value, the huge value, the second request?
Which of these strings came from outside?
What did my prompt not mention?
  1. Line 1Ask it of every function, not every file. If the answer is "the caller checks", find the caller and check.
  2. Line 4The highest-yield question in this course. The output addresses the prompt; the gaps are wherever the prompt was silent, and you are the only person who knows where that was.
Wrenlearner

If I have to check all of that, have I saved any time at all?

Rookmentor

Yes, and less than it felt like. The saving is in typing and in recall, and it is real. What was never saved is the reviewing — and the reason this matters is that the feeling of speed comes from the same fluency that makes review harder.

Magpieadversary

I read the pull requests that went in fast. Not because they are worse, but because "this looks fine" is a decision somebody made in four seconds.

Why does generated code get reviewed less carefully than hand-written code?

Tip

Review generated code at the same speed you would review a stranger's first contribution. Not because it is worse than yours — because you have no memory of writing it, and memory is most of what makes reviewing your own code fast.