Latest security q2

.changeset/boozy-manual-severaltoms.md

@@ -0,0 +1,5 @@
+---
+"sanitize-html": patch
+---
+
+Security: added a number of new attributes to be protected against unsafe URLs, e.g. `javascript:` and similar. None of these are used in the default configuration of `sanitize-html` or `apostrophe` or likely to be used there, and some attributes, like an `action` for a `form`, are inherently unsafe to allow if XSS protection is your goal. Nevertheless it makes sense to block certain URL types where they are not appropriate. Some attributes are not supported at all by modern browsers but are included for completeness. Thanks to [crattack](https://github.com/crattack) for reporting the vulnerability.

.changeset/file-pretty-url-host-header-ssrf.md

@@ -0,0 +1,5 @@
+---
+"apostrophe": patch
+---
+
+Security: when `@apostrophecms/file` pretty URLs are enabled (`prettyUrls: true`), the upstream request used to serve the file is no longer built from the incoming `Host` header. The self-request is now resolved against the site's configured `baseUrl` (via `req.baseUrl`), falling back to the request host only when no `baseUrl` is configured. This closes a server-side request forgery (SSRF) vector in which the `Host` header could steer the proxied fetch at another host. The real-world risk was low: the path is constrained to an existing attachment's `/uploads/attachments/<cuid>-<slug>.<ext>`, and cuids are unique and immutable, so any reachable content was already public via the front door. Thanks to [EchoSkorJjj](https://github.com/EchoSkorJjj) for reporting the issue.

.changeset/proud-moons-guard.md

@@ -0,0 +1,5 @@
+---
+"apostrophe": patch
+---
+
+Security fix: server-side prototype pollution (CWE-1321) via dot-notation paths. `apos.util.set()` and `apos.util.get()` now refuse to traverse `__proto__`, `constructor` and `prototype` path segments. Previously an authenticated editor could send a PATCH REST API request whose patch operators (for example `$pullAll` with a key of `__proto__.publicApiProjection`) wrote to `Object.prototype`. A polluted `publicApiProjection` defeated the `publicApiCheck()` authorization gate on piece-type REST endpoints for subsequent unauthenticated requests, for the lifetime of the Node.js process. All users should update. Thanks to [tonghuaroot](https://github.com/tonghuaroot), [H3xV0rT3x](https://github.com/H3xV0rT3x), and [5h1kh4r](https://github.com/5h1kh4r) for reporting the vulnerability.

.changeset/seo-analytics-xss.md

@@ -0,0 +1,5 @@
+---
+"@apostrophecms/seo": patch
+---
+
+Security: the Google Analytics tracking ID (`seoGoogleTrackingId`) and Google Tag Manager ID (`seoGoogleTagManager`) global SEO fields were interpolated directly into the bodies of inline `<script>` tags without escaping. Any user permitted to edit the global document, including editors and contributors (if their submission were approved), could set these fields to a value that broke out of the surrounding script and executed arbitrary JavaScript for every visitor on every page (stored XSS). These values are now emitted as escaped `json` nodes, matching the JSON-LD handling, so they can no longer terminate the `<script>` element or escape the string literal they sit in. All projects using `@apostrophecms/seo` with untrusted editors should upgrade promptly to close this vulnerability. Thanks to [H3xV0rT3x](https://github.com/H3xV0rT3x) and [hibrian827](https://github.com/hibrian827) for reporting the issue.

packages/apostrophe/modules/@apostrophecms/file/index.js

@@ -248,14 +248,15 @@ module.exports = {
             const uglyUrl = self.apos.attachment.url(file.attachment, {
               prettyUrl: false
             });
-            // For relative URLs (local uploadfs, not CDN), resolve
-            // against the current server's origin so the proxy can
-            // make the self-request.  During static builds
-            // `attachment.url()` may return only a path.
-            const proxyUrl = uglyUrl.startsWith('/')
-              ? `${req.protocol}://${req.get('host')}${uglyUrl}`
-              : uglyUrl;
-            return await streamProxy(req, proxyUrl, { error: self.apos.util.error });
+            // `uglyUrl` may be relative (local uploadfs) or absolute
+            // (S3/CDN). For the relative case `streamProxy` resolves it
+            // against `req.baseUrl`, which reflects the configured
+            // `baseUrl` (or locale hostname) and only falls back to the
+            // request host when the site has none. We deliberately do
+            // not build the upstream URL from the raw `Host` header
+            // here, as that is attacker-controlled and would allow the
+            // self-request to be redirected to an arbitrary host (SSRF).
+            return await streamProxy(req, uglyUrl, { error: self.apos.util.error });
           } catch (e) {
             self.apos.util.error('Error in pretty URL route:', e);
             return res.status(500).send('error');

packages/apostrophe/modules/@apostrophecms/util/index.js

@@ -35,6 +35,13 @@ const util = require('util');
 const { stripIndent } = require('common-tags');
 const glob = require('../../../lib/glob.js');
 
+// Dot-path segments that must never be traversed when walking a
+// user-supplied path in `apos.util.get` and `apos.util.set`. Following any
+// of these reaches the prototype chain and enables server-side prototype
+// pollution (CWE-1321), e.g. a PATCH `$pullAll` key of
+// `__proto__.publicApiProjection`.
+const unsafePathSegments = new Set([ '__proto__', 'constructor', 'prototype' ]);
+
 module.exports = {
   options: {
     alias: 'util',
@@ -755,6 +762,10 @@ module.exports = {
             if (o == null) {
               return undefined;
             }
+            if (unsafePathSegments.has(p)) {
+              // Never read through the prototype chain (CWE-1321)
+              return undefined;
+            }
             o = o[p];
           }
         }
@@ -819,6 +830,12 @@ module.exports = {
           }
         }
         path = path.split('.');
+        for (p of path) {
+          if (unsafePathSegments.has(p)) {
+            // Refuse to write through the prototype chain (CWE-1321)
+            throw self.apos.error('invalid', `Unsafe property name "${p}" in dot path`);
+          }
+        }
         for (i = 0; (i < (path.length - 1)); i++) {
           p = path[i];
           o = o[p];

packages/apostrophe/test/files.js

@@ -132,6 +132,34 @@ describe('Files', function() {
     }
   });
 
+  it('should ignore a spoofed Host header when proxying the pretty URL (SSRF regression)', async function() {
+    const req = apos.task.getAnonReq();
+    try {
+      apos.file.options.prettyUrls = true;
+      const files = await apos.file.find(req).toArray();
+      assert.strictEqual(files.length, 1);
+      const file = files[0];
+      const attachment = apos.attachment.first(file);
+      const url = apos.attachment.url(attachment);
+      assert(url);
+      // Send an attacker-controlled Host header (e.g. the cloud metadata
+      // address from the advisory). The upstream fetch must be resolved
+      // against the server-trusted baseUrl, not this header, so the
+      // legitimate content is still served and the request is never
+      // steered at the spoofed host.
+      const response = await apos.http.get(url, {
+        headers: {
+          Host: '169.254.169.254'
+        },
+        fullResponse: true
+      });
+      assert.strictEqual(response.status, 200);
+      assert.strictEqual(response.body, attachment.data);
+    } finally {
+      apos.file.options.prettyUrls = false;
+    }
+  });
+
 });
 
 describe('Files with i18n locale prefixes', function() {

packages/apostrophe/test/utils.js

@@ -372,6 +372,109 @@ describe('Utils', async function() {
       assert(data.shoes[0].size === 8);
     });
 
+    // Server-Side Prototype Pollution (CWE-1321) regression coverage.
+    // apos.util.set and apos.util.get traverse user-supplied dot-notation
+    // paths. A segment of `__proto__`, `constructor` or `prototype` must
+    // never be followed, or an authenticated editor could write to
+    // Object.prototype (for example via the $pullAll patch operator) and
+    // poison authorization checks process-wide. See GHSA-6h5j-32cf-4253.
+
+    it('utils.set must reject a __proto__ segment instead of polluting Object.prototype', function() {
+      const data = {};
+      try {
+        assert.throws(() => {
+          apos.util.set(data, '__proto__.polluted', 'yes');
+        }, { name: 'invalid' });
+        assert.strictEqual({}.polluted, undefined);
+        assert.strictEqual(data.polluted, undefined);
+      } finally {
+        // Belt and suspenders: if the guard ever regresses, do not leak a
+        // polluted prototype into the rest of the suite.
+        delete Object.prototype.polluted;
+      }
+    });
+
+    it('utils.set must reject a constructor.prototype segment', function() {
+      const data = {};
+      try {
+        assert.throws(() => {
+          apos.util.set(data, 'constructor.prototype.polluted', 'yes');
+        }, { name: 'invalid' });
+        assert.strictEqual({}.polluted, undefined);
+      } finally {
+        delete Object.prototype.polluted;
+      }
+    });
+
+    it('utils.set must reject a trailing __proto__ segment rather than replacing the prototype', function() {
+      const data = {};
+      assert.throws(() => {
+        apos.util.set(data, '__proto__', { polluted: 'yes' });
+      }, { name: 'invalid' });
+      assert.strictEqual(Object.getPrototypeOf(data), Object.prototype);
+      assert.strictEqual(data.polluted, undefined);
+    });
+
+    it('utils.set must reject a dangerous segment exposed after an @ reference', function() {
+      const data = {
+        items: [
+          {
+            _id: 'abc',
+            sub: {}
+          }
+        ]
+      };
+      try {
+        assert.throws(() => {
+          apos.util.set(data, '@abc.__proto__.polluted', 'yes');
+        }, { name: 'invalid' });
+        assert.strictEqual({}.polluted, undefined);
+      } finally {
+        delete Object.prototype.polluted;
+      }
+    });
+
+    it('utils.get must not traverse into the prototype chain via __proto__', function() {
+      // Without the guard this returns Object.prototype.toString (a function).
+      assert.strictEqual(apos.util.get({ a: 1 }, '__proto__.toString'), undefined);
+      assert.strictEqual(apos.util.get({ a: 1 }, '__proto__'), undefined);
+    });
+
+    it('implementPatchOperators must not let a $pullAll key pollute Object.prototype', function() {
+      // The reported attack vector: an authenticated editor PATCH body of
+      // { $pullAll: { '__proto__.publicApiProjection': [] } } reached
+      // apos.util.set with a fully attacker-controlled key.
+      const patch = {
+        $pullAll: {
+          '__proto__.publicApiProjection': []
+        }
+      };
+      try {
+        assert.throws(() => {
+          apos.schema.implementPatchOperators(patch, {});
+        }, { name: 'invalid' });
+        assert.strictEqual({}.publicApiProjection, undefined);
+      } finally {
+        delete Object.prototype.publicApiProjection;
+      }
+    });
+
+    it('implementPatchOperators must not let a direct dot-notation key pollute Object.prototype', function() {
+      // The second documented entry point: a top-level dotted key whose value
+      // is fully attacker-controlled.
+      const patch = {
+        '__proto__.publicApiProjection': { title: 1 }
+      };
+      try {
+        assert.throws(() => {
+          apos.schema.implementPatchOperators(patch, {});
+        }, { name: 'invalid' });
+        assert.strictEqual({}.publicApiProjection, undefined);
+      } finally {
+        delete Object.prototype.publicApiProjection;
+      }
+    });
+
     it('should slugify', function () {
       // Basic
       assert.equal(

packages/create-apostrophe/src/ui/links.js

@@ -14,12 +14,12 @@ const LINKS = Object.freeze({
 
 /** Per-kit "get oriented" guides. `*-demo` and `*-demo-data` share a guide. */
 const KIT_GUIDES = Object.freeze({
-  'apostrophe-astro-essentials': 'https://apostrophecms.com/guides/astro-essentials-overview',
-  'apostrophe-astro-demo': 'https://apostrophecms.com/guides/astro-demo-overview',
-  'apostrophe-astro-demo-data': 'https://apostrophecms.com/guides/astro-demo-overview',
-  'apostrophe-essentials': 'https://apostrophecms.com/guides/apostrophe-standalone-essentials-overview',
-  'apostrophe-demo': 'https://apostrophecms.com/guides/apostrophe-demo-overview',
-  'apostrophe-demo-data': 'https://apostrophecms.com/guides/apostrophe-demo-overview'
+  'apostrophe-astro-essentials': 'https://apostrophecms.com/docs/guide/astro-essentials-overview.html',
+  'apostrophe-astro-demo': 'https://apostrophecms.com/docs/guide/astro-demo-overview.html',
+  'apostrophe-astro-demo-data': 'https://apostrophecms.com/docs/guide/astro-demo-overview.html',
+  'apostrophe-essentials': 'https://apostrophecms.com/docs/guide/apostrophe-standalone-essentials-overview.html',
+  'apostrophe-demo': 'https://apostrophecms.com/docs/guides/apostrophe-demo-overview.html',
+  'apostrophe-demo-data': 'https://apostrophecms.com/docs/guide/apostrophe-demo-overview.html'
 });
 
 /** @typedef {keyof typeof LINKS} LinkName */

packages/db-connect/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@apostrophecms/db-connect",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "description": "Database connection library and dump/restore tools for ApostropheCMS",
   "license": "MIT",
   "main": "index.js",

packages/sanitize-html/README.md

@@ -182,7 +182,14 @@ selfClosing: [ 'img', 'br', 'hr', 'area', 'base', 'basefont', 'input', 'link', '
 // URL schemes we permit
 allowedSchemes: [ 'http', 'https', 'ftp', 'mailto', 'tel' ],
 allowedSchemesByTag: {},
-allowedSchemesAppliedToAttributes: [ 'href', 'src', 'cite' ],
+allowedSchemesAppliedToAttributes: [
+  'href', 'src', 'cite',
+  'action', 'formaction', 'data', 'xlink:href',
+  'poster', 'background', 'ping',
+  'longdesc', 'usemap', 'codebase', 'classid', 'archive',
+  'profile', 'manifest', 'itemid',
+  'dynsrc', 'lowsrc'
+],
 allowProtocolRelative: true,
 enforceHtmlBoundary: false,
 parseStyleAttributes: true

packages/sanitize-html/index.js

@@ -440,11 +440,11 @@
                 return;
               }
             }
-            if (a === 'srcset') {
+            if (a === 'srcset' || a === 'imagesrcset') {
               try {
                 let parsed = parseSrcset(value);
                 parsed.forEach(function(value) {
-                  if (naughtyHref('srcset', value.url)) {
+                  if (naughtyHref(a, value.url)) {
                     value.evil = true;
                   }
                 });
@@ -579,7 +579,7 @@
         // double-encoding. Other "nonTextTags" like <option> are not raw text
         // elements in htmlparser2, so their contents are decoded and must be
         // escaped below like any other text (important to prevent XSS via
         // entity-encoded payloads such as <option>&lt;script&gt;...&lt;/script&gt;</option>).
         result += text;
       } else if (!addedText) {
         const escaped = escapeHtml(text, false);
@@ -957,7 +957,14 @@
   // URL schemes we permit
   allowedSchemes: [ 'http', 'https', 'ftp', 'mailto', 'tel' ],
   allowedSchemesByTag: {},
-  allowedSchemesAppliedToAttributes: [ 'href', 'src', 'cite' ],
+  allowedSchemesAppliedToAttributes: [
+    'href', 'src', 'cite',
+    'action', 'formaction', 'data', 'xlink:href',
+    'poster', 'background', 'ping',
+    'longdesc', 'usemap', 'codebase', 'classid', 'archive',
+    'profile', 'manifest', 'itemid',
+    'dynsrc', 'lowsrc'
+  ],
   allowProtocolRelative: true,
   enforceHtmlBoundary: false,
   parseStyleAttributes: true,