Two security rules I wrote and deleted, and why the second one was worse

A security ruleset is judged by what it does not flag. Anyone can write a pattern that catches a vulnerability. The difficulty is catching it without also catching the ten correct implementations that look similar.

I keep a REJECTED.md in my ruleset for rules that were written, tested, and cut. Here are the two most instructive.

One: taint analysis cannot see a guard

The rule was going to flag a column name taken from the request and passed to orderBy. This is a real vulnerability. Column names cannot be bound as parameters the way values can, so the only correct fix is an allowlist.

The correct fix looks like this:

$sortable = ['created_at', 'total', 'name'];
abort_unless(in_array($request->sort, $sortable, true), 400);

$query->orderBy($request->sort, 'desc');

That code is safe. The rule fired on it anyway, exactly as loudly as on the broken version.

The cause is structural. Taint analysis tracks values. It knows $request->sort is tainted and watches for that tainted value reaching a sink. A sanitizer takes a tainted value and returns a clean one, so the analysis can follow the new value.

in_array does not do that. It inspects its argument and returns a boolean. The dangerous value is still sitting in $request->sort, unchanged, on the next line. What made the code safe was a control flow decision about the program, which the analysis does not model.

Semgrep has by-side-effect: true for sanitizers that clean in place, which is what this case looks like it needs. It does not clear it here, verified against a minimal two function fixture.

So I cut it. A rule that flags correct, idiomatic framework code teaches people to skim the report, and the skimming does not stop at the false positive.

What would make it shippable is a narrower rule that only fires when the enclosing function contains no membership check at all. That is expressible in principle, and Semgrep's PHP parser rejected the pattern-not-inside function form it needs. Until then it belongs in a code review checklist, where a human can weigh the surrounding lines.

Two: the rule that was just wrong

This one shipped.

The rule flagged Laravel's mimetypes: validation rule at ERROR severity, with a message saying it trusts the Content-Type header the client sent, while mimes: inspects the actual file. Four characters apart, opposite guarantees. It made a satisfying example, and it was the flagship rule and the opening of the README.

It is also false.

// Illuminate\Validation\Concerns\ValidatesAttributes
public function validateMimes($attribute, $value, $parameters)      { ... $value->guessExtension() ... }
public function validateMimetypes($attribute, $value, $parameters)  { ... $value->getMimeType() ... }

UploadedFile does not override getMimeType(), so both resolve to File::getMimeType(), which is MimeTypes::getDefault()->guessMimeType($path). Both inspect the file. Neither reads the header. The accessor that returns the header is getClientMimeType(), and no validation rule calls it.

So a security tool was flagging correct code at high confidence and telling people a safe validation rule was a vulnerability. That is worse than the first case in a specific way. A noisy rule wastes attention. A wrong rule moves people away from a good practice, and someone could reasonably have replaced mimetypes: with something weaker on my advice.

I found it by accident, checking whether "laravel mimes vs mimetypes" was a search term worth targeting, and noticing that the results disagreed with my own README.

What I took from both

Reading the source beats reading summaries of it, including confident summaries and including your own memory of how a framework works. The check took ten minutes.

A rule's message is a claim and needs the same checking as the pattern. I tested that the pattern matched what I intended. I never tested that what I intended was true, and those are different activities.

Publish the cut ones. A ruleset with no rejected rules either has not been tested against real code, or is not telling you about it.

The ruleset is stacksec, for Laravel and Next.js, and REJECTED.md is where both of these live in full.