Audit 0.20.0 on 103 projects: crash and hang fixes, security checks, precision caps - #26
Merged
Merged
Conversation
A byte step left the splitter inside a multibyte character, and the next slice panicked: pgweb's booktown.sql, which holds U+FFFD outside quotes, aborted the whole check with exit 101.
A Python block keeps its trailing comment, and a block's last byte fell inside the `线` closing one in vnpy's scripts: slicing the source there panicked and aborted the check. The SQL splitter's own line helper had the same slice for a file ending on such a character.
Source files that are binary or not UTF-8 were skipped, but a document was an error: one Markdown file holding NUL bytes in dvja's docs made the whole check incomplete (exit 2).
Every security unit looked up its callers and the errors its callees raise by testing every selected file's imports again, and a C# file's imports are all its lines of code: jellyfin's dry run ran past half an hour and now takes 17 seconds, and laravel/framework's took 254 seconds and now takes 88. Links keeps each file's importers and the files it reaches as first asked, and a name made of segment characters is looked up among a file's segments instead of searched for in every line. The requests are unchanged on all 95 corpus projects.
…the owner's choice PostgreSQL lets every role execute a new function, but an empty list of grants and revokes said nothing: chatbot-ui's delete_storage_object, which deletes any stored file with the service role key and which nothing revokes, stayed at 0.68 on skipping the caller check and was only a search_path consider. A policy that lets others read rows their owners marked shared or public (`sharing <> 'private'`) is the read side of sharing; ten such policies were considers.
oak writes its 266 tests as Deno.test({ name, fn() {…} }), and neither
the test spans nor the test map knew the call: its test files each got a
file-purpose request and the test rules had nothing to judge. The string
and named-function forms, and .only and .ignore, are cases too.
…ain-text passwords Code outside C# and Django is asked the token and key checks C# asks: a JWT decoded without its signature scored 0.93 to 0.98 as a weakened setting in DVGA and JavaVulnerableLab, and with no check to name it was only a note; DVNA's session secret 'keyboard cat' and RailsGoat's encryption key were missed. Every language is asked whether code turns off HTML escaping (NodeGoat's autoescape: false, RailsGoat's escape_html_entities_in_json = false). The password check asks about passwords saved or checked as plain text, which NodeGoat and JavaVulnerableLab do and no check named. A cookie added to a request or set empty and expired to delete it is not a session cookie without flags: shiori's three such cookie reviews were wrong. WordPress's wp_rand and wp_generate_password are cryptographic.
… the code does Asked whenever their check is not clear, each can only clear it: what code does with tokens (front-end hooks that send their own token and middleware that looks a session up stayed between 0.2 and 0.5 on the new token check), how it handles users' passwords (HMAC signing was a password-hash review), and what its logs write, now with who did what and output shown on purpose to a command-line tool's operator apart from other personal data: 10 of 19 labeled logging reviews were audit lines naming who signed in or commands printing recovery codes for the admin who ran them. A Choice asked whenever its check is not clear also clears a presence answer or check that found a concern it rules out.
Considers that rest only on whether a value needs a descriptive name were right 52% of the time on 35 hand-labeled projects: 19 of 52 when the file writes the value once, 34 of 49 when it repeats it. A delay given to setTimeout, a size given to an attribute or a CSS class reads where it is used; a value written twice can drift apart. Such a consider is now a note that says why, and reviews stay. On the corpus, 156 considers became notes and labeled hardcoded-value considers went from 51% to 65% right. The count is made at planning and asks nothing.
… only rows chatbot-ui's delete_storage_object deletes any stored object through the storage API with the service role key and nothing revokes EXECUTE on it, so anyone can delete any file. Asked about rows only, it stayed at 0.75 on skipping the caller check; asked about rows or stored files, it and delete_storage_object_from_bucket are reviews at 0.95, and no other access-control finding changed on six Supabase projects.
…s client input A graphene resolver's arguments read as parameters any caller could pass: DVGA's SQL injection in resolve_pastes and SSRF in ImportPaste::mutate were considers "if a caller passes outside input", and its command injection in resolve_system_debug a note. A Python file that imports graphene, strawberry or ariadne now carries a framework note, as Next.js and SvelteKit files do, and all three are reviews. Only such files' requests change.
…ms from trusting it A file at a test path that holds no tests is judged as application code, and the dummy apps of devise and clearance and a test model hashing with password.reverse were three wrong reviews, the only security reviews or considers at test paths across 103 projects. Such findings are one level lower, like code that runs only in development. The token Choice separates decoding a token to read what it says (an expiry, a user or character id) from deciding access with it: code that read the expiry of a token its identity provider had just sent, or the character id of its own access token, was chosen as trusting a token unverified, while every labeled true token finding decided access with it or turned verification off.
…eviews too, and keep only turned-off token checks at review An instruction section of fewer than 15 tokens is a note: labeled by hand, 1 of 10 findings on such sections was right, most of them a title and a Last updated line read as a record of past work (about 35 of one repository's considers), against 64 of 68 on larger sections. Where a function's text goes is asked whenever its error-detail signals are not clear, and can clear a check that found a concern: a game client handing the server's error text to its own window over a channel whose messages are named Response was fifteen reviews for sending details to a remote client. A review that only the token check names is a consider unless the code turns a library's verification off: whether a token was verified before the function reads it lies outside the function, and such reviews were right in intentionally vulnerable apps and wrong in three others.
Naming a value is a cleanup, as comments and instruction sections are. Labeled by hand, 17 reviews that rested only on whether a value needs a descriptive name were right and 18 wrong, most of the wrong ones tuning in game, audio and animation code: a scheduler's 500 ms, a hash seed, a mix gain, a float epsilon. Such findings are at most a consider, and still a note when their file writes the value once. Findings about a value that differs between deployments or special-cases one identity keep their level.
…hoice A game client hands the server's error text to its own window over a channel of Response messages; asked where the text goes, it was answered as a response to a client, and the fifteen reviews for sending internal details to a remote client stayed. The local option names the screens a desktop, game or mobile app reaches through a channel, event or IPC call, and the client option a connected remote client.
A directory named deprecated, archive, attic, retired, obsolete or proof of concept marks its code as a function marked deprecated does: it is not worth sharing code with. A Unity project's Assets/ProofOfConcept builders, kept as a reference with no menu entry, were paired with the live scene builders in six wrong reviews. legacy is left out, since legacy code is often still served. No other corpus project has such a directory.
A password check or token check at review needs its Choice to name what the function does: hashing with a fast hash, or turning a library's verification off. Whether a callee or an entity's @BeforeInsert hook hashes the password a function saves, or whether a token was verified before the function reads it, lies outside the function: dvja's register and edit, which pass the password to a service that hashes it, and nest-realworld's create, whose entity hook hashes it with argon2, were plain-text reviews. Such findings are considers now, as unverified-token reviews already were.
A game client hands the server's error text to its own window over a channel of Response messages, and asked where the text goes the answer was a remote client, in 16 wrong reviews for sending internal details. A file whose package depends on an interface toolkit (ratatui, egui, iced, bevy, tauri, electron, spacetimedb-sdk and others) now carries a framework note saying the program runs on its user's machine, as Next.js and SvelteKit files do; all 16 are gone. Only such packages' requests change: four of the corpus's 103 projects, about $0.20.
Markdown under .claude/skills, .claude/commands and .claude/agents was not read at all: one project's skill cited documentation paths that a rename had removed. A session loads only their descriptions and reads the rest when one is used, so they are project documentation, checked for stale paths, repetition and size, and do not count toward what loads at a session's start. A path holding a $ placeholder, such as a command's .kiro/specs/$1/spec.json, names no file.
A file under a directory whose name holds a {{ … }} placeholder, as a
cookiecutter template's {{cookiecutter.project_slug}} does, held Jinja
statements and placeholders that are no syntax of its language: 31 of
cookiecutter-django's Python and JavaScript files, the generated
application's settings, models, views and tests, were skipped for
syntax errors. Such files are parsed with their statements and comments
blanked and each placeholder read as a name of the same length, so byte
offsets and lines stay the file's and the evidence keeps the tags. All
31 are judged; the Celery settings that turn off Redis certificate
checks are a review. No other corpus project has such a directory.
… test setup JevGate's review of this branch found Tally::add's chain of caps hiding its main path and the new unsafe-settings tests repeating their setup. The caps are one function with early returns, and the tests run through one helper. No finding changes on the corpus's 103 projects.
The questions file held the presence questions, the checks and the settle Choices, which JevGate's review found could be read apart. The Choices move to questions/settle.rs with their text unchanged; no request or finding changes on the corpus.
…atching_windows When an outline's split Score stays undecided, the kind of file decides. The same kind of code written out per feature counted as serving several features and raised a consider; every such consider was wrong: vaultwarden's mailer, a game's admin reducers per kind of map content, and the settle questions JevGate's self-review flagged after they moved to their own module. The kind now clears, and undecided file outlines on the corpus went from 106 to 45. Nothing is asked again. matching_windows walks seed pairs from an iterator of its own, which the self-review found hidden under three loops; every corpus project's requests are unchanged.
The split Score reads a large document's headings alone, and in every labeled split finding it took one subject for several: dated release plans, READMEs, the RealWorld frontend instructions and a list of business rules. The kind, asked until now only when the split stayed undecided, is asked of split findings too, and gains a plan for one change and requirements; a kind that serves one subject clears the finding before its part is located. On the corpus 14 split considers are gone, for about $0.002 of kind requests. The large-docs note says a document "may mainly record" past work.
Planning built every unit's trace, recheck and settles up front and held them as JSON values; most are never sent. On laravel/framework's 3,000 PHP files the security follow-ups came to 580 MB of JSON, and a dry run peaked at 3.2 GB for the security rules and 4.1 GB for all rules. A FollowUp keeps the request's text and reads it back when the follow-up is asked: the peaks are 1.5 GB and 2.2 GB. serde_json reads the text back to the same value, and a run of all 103 corpus projects paid for no request.
A comment undecided after its first pass and recheck is asked what kind of comment it is, and only a kind at 0.80 decided it. Step headings such as `// update any single tag` above `this.addTag()` stayed between the thresholds on every ask: 1,153 comments on the corpus, each leaving its file uncertain. The kind is the last ask, so a leaning kind now decides: toward a kind a reader could do without, a note, else clear. Undecided units on the corpus went from 2.24% to 1.50% of judged units; nothing is asked again.
The internal-details check sees a test and the signatures it calls, and read a debug panel's recorded queries (`panel._queries`, which the panel renders), Devise's documented hooks and an app's state after an action as internals: 49 of 66 such considers labeled on the corpus were wrong. What the assertions read separated them: none of the 44 reading state or effects was right, and 16 of the 19 reading stored input or the program's own calls were. A consider from that check is now asked, with the recheck's evidence, what its assertions read: results, state or effects clear it, stored input or own calls keep it, and a split answer leaves a note. A test that reads members through reflection or a cast to `any` keeps its consider unasked: the question read those as results. The recheck's request is unchanged. Labeled tests/value considers went from 29% to 77% right, and on held-out projects from 2 right and 13 wrong to 1 wrong, for about $0.006.
The probe for instruction files that ignore files hide asked whether `AGENTS.md` exists in each directory. On a case-insensitive file system that also opens `agents.md`: refined-github keeps its instructions there, and a React Native template keeps `claude.md`. The file was then read under the probed name, the canonical path differed, the read failed as a symlinked path, and the whole run ended incomplete with exit 2. The probe now requires a directory entry of exactly that name.
A symlinked instruction file loads its target's text, but only when the target was itself an instruction file by name. refined-github's `CLAUDE.md` links to `agents.md`, so Claude Code's instructions were neither judged nor counted toward what a session loads. Link targets are read too, and a target no harness reads by its own name takes the link's readers: its findings name the file itself and its text is counted once. Requests change only on the two corpus projects with such files.
Copies between packages with no local dependency path between them are not compared, but only Node, Rust and Python manifests named a package. Online Boutique's Go services are each their own module, and 9 of the 10 copies found between them, labeled on this audit's fresh projects, were wrong. A module's path is its name and its `require` lines its dependencies, so modules that share a local module are still compared.
…, and share the linked-file test
JevGate never read server templates, and they held most of the documented vulnerabilities it missed in the intentionally vulnerable apps. - An ERB, EJS, JSP, Handlebars, Mustache, Nunjucks, Twig, Jinja or Go template, or HTML under templates/, views/, layouts/, partials/ or includes/, is parsed as its inline scripts with its tags blanked, and its top-level script is judged like a PHP page script. Its requests say the code runs in the visitor's browser. - Its `template code` is one more unit: tags that write request, cookie, session or signed-in-user data unescaped (judged by injection), and every scriptlet and declaration of a JSP page once one reads the request (judged by every security rule). - A Node handler that renders a view by name is sent the view's unescaped lines, as Django views are, and its questions name such templates. A template holding neither inline scripts nor template code is not selected. On the corpus, 24 documented vulnerabilities are found that no rule read (RailsGoat's and DVNA's XSS, DVGA's paste-page XSS, DVJA's JSP XSS, JavaVulnerableLab's JSP-only injections, traversal, SSRF and leaked stack traces); 42 of 49 labeled reviews and 16 of 25 considers in templates were right. Requests outside templates and render calls are unchanged on the other 101 projects.
…check's help
Pug writes a value unescaped with `!=` right after a tag or at the start of
a line, and with `!{…}`; `err.name !== 'AbortError'` in a view's script
is a comparison. hackathon-starter's handlers were sent such comparisons as
unescaped output. `check --help` names the component and server template
files JevGate selects.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An audit of 0.20.0 against 103 pinned projects: the existing corpus, 24 open-source projects of kinds not tried before (intentionally vulnerable Rails, Node, GraphQL, C# and Java apps, a Deno framework, a WordPress plugin, a cookiecutter template, Angular, a Rocket server, Go web apps, and projects in Kotlin, Swift, Elixir and C), and 8 more of the maintainer's own. Findings were labeled by hand against the code.
Results
The fresh projects are 9 more of the maintainer's own repos and 5 open-source kinds not tried before: Online Boutique (polyglot gRPC microservices), refined-github (a browser extension), a React Native template, Uniswap v2 (Solidity) and a dbt project. Both versions' findings were labeled by hand after the other changes here were made. Only the
go.modfix and the two instruction-file fixes came from them (Online Boutique, refined-github). Measured with #27's head, which contains this branch.No regression on the 0.20.0 corpus run: before these changes, 0.20.0 reproduced the previous audit's run exactly on all 71 projects. JevGate's full self-check with this branch's binary leaves only the baselined
visitconsider.Fixes
agents.mdorclaude.mdwas probed asAGENTS.md/CLAUDE.md, failed to read, and made the whole run incomplete (exit 2): refined-github and a React Native template, both new to the corpus. Names now match exactly, and aCLAUDE.mdthat links to such a file loads it as Claude Code's instructions.go.modis read as a package manifest, so copies between Go modules that don't depend on each other are not compared. Online Boutique's services are separate modules, and 9 of the 10 copies found between them were wrong.Findings
delete_storage_object(anyone can delete any file) becomes a review. Rows owners mark shared are acceptable.as anykeeps its consider.Deno.testcases.matching_windowswalks seed pairs from an iterator. No finding changes on the corpus.For your review
Policy calls:
CI's
reviewjob runs the released 0.20.0, which still raises two "same kind of code for several features" considers (settle.rs,test_rules.rs) that this branch's file-organization change removes. The branch's own self-check leaves only the baselinedvisit.The changelog has every change with its numbers. Measured-but-rejected: a rewording of the unnamed-value question (see
evaluation/experiments, local).