Fix various security issues #1
Loading…
Reference in a new issue
No description provided.
Delete branch "fix-security-issues"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
CVE-2026-71481: verifySolution skipped the HMAC check whenever the challenge carried no signature, so a caller could submit a challenge it had fabricated itself - no signature, cost 1 - solve it instantly, and pass verification. That made every captcha-gated endpoint (register, login, contact, /get-download) fully automatable. - Upgrade altcha-org/altcha to ^2.0.3, which fixed this ("reject missing challenge signature when secret is set"). Installed v2.1.0. - Also reject a missing signature inside the rule, so the property holds even with no secret configured: the library skips signature verification entirely in that case, which would silently reopen the bypass. - Require ALTCHA_HMAC_KEY in .env.example. Without it challenges are issued unsigned, and captcha-gated forms now fail closed instead of degrading. Verified with fixtures in tests/Support/AltchaPayload that reproduce the reported payload shapes (self-issued, stripped signature): 3 of the 4 cases fail against the vulnerable 2.0.2 and all pass on the fixed version. Full suite: 200 passed.PlaylistSidebar::$isOwner is a public Livewire property. The snapshot is HMAC-signed, but the `updates` object of POST /livewire/update is not, and Livewire applies it to any public property, so a caller could simply send `updates: {isOwner: true}`. The component is rendered on the public stream page, so an anonymous visitor could do this with a snapshot taken from any accessible `?playlist=` URL. remove() then looked the entry up globally, so the flipped flag let that visitor delete any playlist_episodes row - including entries of private playlists - and an owner could delete entries belonging to other playlists. - Mark $isOwner #[Locked], which rejects client-side updates to it. - Scope the remove() lookup to the mounted playlist, so the action cannot reach another playlist even if the flag is ever wrong again. - Extract the snapshot-replay helper into Tests\Support\LivewireSnapshot and reuse it from the admin authorization test. Verified: both attack cases fail against the unfixed component (the guest replay really does delete the victim's row) and pass after the fix, while the owner can still remove their own entry. Full suite: 203 passed.The VideoObject block emitted json_encode($data, JSON_UNESCAPED_SLASHES) into a <script type="application/ld+json"> element. json_encode does not escape < or >, and JSON_UNESCAPED_SLASHES keeps "/" literal, so an episode title containing "</script>" closed the script element early. Titles are moderator-writable (POST /admin/episode/edit -> updateTitle), so a moderator could run script for every visitor of the episode page, admins included, and from there drive /admin/* with the admin's own CSRF token. Adding JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT escapes those characters as \uXXXX, which both prevents the early close and keeps the block valid JSON-LD for crawlers. Note: Js::from() must not be used here - it emits a JavaScript expression (JSON.parse('...')), not JSON, which would break the structured data. Verified: with the payload in a title the block is truncated and fails to parse before the fix, and parses with the full title restored after it. Full suite: 205 passed.Watched::$userId was an untyped public property set in mount() from the authenticated user. The snapshot is signed but the `updates` object of POST /livewire/update is not, so any signed-in user could send `updates: {userId: <someone else>}` and page through that account's viewing history. Marking it #[Locked] and typing it as int makes Livewire reject client-side updates to it; mount() still assigns it server-side. Verified: before the change the tampered request returns 200 with the victim's episodes rendered; after it the update is rejected and none of the victim's data is returned. A companion test asserts the viewer still sees their own history, so the assertion cannot pass just because the page is broken. Note for future tests here: the rejection is a 500 whose Ignition debug page echoes the calling source, so the test disables app.debug before checking the body.Hentai::isLoliOrShota() cached under "episode:{id}:has_problematic_tags", the same key Episode::isLoliOrShota() uses. Hentai ids and Episode ids are separate sequences that overlap, so whichever entity was resolved first decided the answer for the other one holding that integer id, for the full 24 hour TTL. In the dangerous direction a tagged series is treated as safe and guests are served content the login gate is meant to withhold. The key is now namespaced per model. Swept every other Cache::remember key in app/ for the same mistake: the remaining keys are either global literals or prefixed per entity, and this was the only collision. Verified: with a clean episode and a Loli-tagged series forced to share id 4242, the series reported false before the change and true after it. Full suite: 210 passed.The controller branched on the broker status: unknown addresses produced withErrors(['email' => __('passwords.user')]) ("We can't find a user with that e-mail address"), and only known ones flashed a status. Both are rendered by auth/forgot-password.blade.php, so the form told anyone whether an address has an account. The response is now the same either way: a reset link is still sent when the address belongs to an account, and the generic status is flashed regardless. The trade-off is that a throttled request (second request within 60s) shows the same generic message even though no mail is sent - the usual privacy trade-off. Not fixed here: register still returns a unique-rule validation error for an address that already exists, which is the same class of oracle. Its wording is not the problem - the presence of the error is - so closing it means deferring the answer behind an email verification step rather than changing a message. Logged in SECURITY_FIX_PLAN.md for a decision. Verified: the unknown-address case fails before the change and passes after, with the known-address case unchanged. Full suite: 212 passed.Two blade templates placed user-controlled strings inside single-quoted JavaScript literals. HTML escaping does not escape quotes for JS, and addslashes() does not escape newlines: - profile/partials/user-playlists.blade.php interpolated the playlist name raw, so a name containing a quote closed the literal and the rest of the expression was parsed as code (self-XSS on the owner's own playlist page). - livewire/admin-user-search.blade.php used addslashes() for the display name in the confirm dialogs. The quote was handled, so this is not executable, but a name containing a newline still breaks the expression and takes the admin user table's buttons with it. Both now concatenate Js::from(), which emits a JSON-derived literal. Verified by rendering with the name "O'Brien\n\"x\" </script>": the old output contained a literal newline (and, for the playlist, 'O'Brien closing the string); the new output is 'O\u0027Brien\n\u0022x\u0022 \u003C\/script\u003E' in both places. Also swept the other '<lit> {{ }} <lit>' JS interpolations in resources/views - sortBy(), removeRoleFilter() and toggleOption() take hard-coded arrays or values already intersected with allowedRoles, so they are not injectable. Full suite: 212 passed.