Security audit.

lib/clients.js

@@ -72,9 +72,11 @@ const toOidcMetadata = (row) => {
         response_types:             JSON.parse(row.response_types),
         token_endpoint_auth_method: row.token_auth_method
     };
-    if (row.scope) {
+    // Cap every client at the scopes it registered; default to the minimal
-        meta.scope = row.scope;
+    // "openid" so a client that registered no scope cannot silently receive
-    }
+    // email/profile/groups (oidc-provider intersects requested scopes against
+    // this). Broader access must be granted explicitly at registration.
+    meta.scope = row.scope || "openid";
     if (row.token_auth_method !== AUTH_NONE && row.secret_ciphertext) {
         meta.client_secret = idpCrypto.openText({
             ciphertext: row.secret_ciphertext,

lib/constants.js

@@ -5,7 +5,7 @@
 // crypto.js; protocol/policy values live here.
 
 const PLUGIN_NAME    = "saltcorn-idp";
-const PLUGIN_VERSION = "0.0.5";
+const PLUGIN_VERSION = "0.0.7";
 
 // Public OIDC/OAuth2 + machine endpoints live under this path and are
 // CSRF-exempt. Admin (browser, CSRF-protected) pages live under ADMIN_BASE_PATH.

lib/oidc/discovery.js

@@ -25,7 +25,15 @@ const issuerForReq = (req) => {
         // getState unavailable; fall back to request-derived host
     }
     if (!base) {
-        base = req.protocol + "://" + req.get("host");
+        const host = req.get("host") || "";
+        // Defensive: only a clean single host[:port] may seed the issuer in the
+        // fallback path. A forged/garbage Host (multiple values, schemes, control
+        // chars) must never be reflected into the advertised issuer/endpoints.
+        // (A valid-but-wrong Host still requires the operational fix: set base_url.)
+        if (!/^[A-Za-z0-9.-]+(:[0-9]{1,5})?$/.test(host)) {
+            throw new Error(`${constants.PLUGIN_NAME}: refusing to derive issuer from malformed Host "${host}"; set base_url`);
+        }
+        base = req.protocol + "://" + host;
         // eslint-disable-next-line no-console
         console.warn(`[${constants.PLUGIN_NAME}] base_url not set; deriving issuer from request Host (${base}). Set base_url to prevent Host-header issuer poisoning.`);
     }

lib/oidc/provider.js

@@ -67,6 +67,11 @@ const buildProvider = async (issuer) => {
             groups:  ["groups"]
         },
         scopes: ["openid", "email", "profile", "groups"],
+        // Require PKCE for ALL clients (oidc-provider only mandates it for public
+        // clients by default). For a security-first IdP this removes code
+        // interception/replay as a single-factor risk even if a confidential
+        // client's secret leaks.
+        pkce: { required: () => true },
         features: {
             devInteractions: { enabled: false }
         },

lib/saml/idp.js

@@ -166,6 +166,11 @@ const buildSp = (entityID, acsUrl, opts) => {
     opts = opts || {};
     const settings = {
         entityID: entityID,
+        // Sign BOTH the Response envelope AND the Assertion element. Signing the
+        // assertion (not just the message) gives per-assertion integrity and is
+        // the posture least exposed to XML-signature-wrapping at the consuming SP.
+        wantMessageSigned:    true,
+        wantAssertionsSigned: true,
         assertionConsumerService: [
             { Binding: saml.Constants.namespace.binding.post, Location: acsUrl }
         ]

lib/saml/routes.js

@@ -138,7 +138,10 @@ const expandGroupValues = (template, groupsArr) => {
             values += "<saml:AttributeValue" + attrs + ">" + escapeXmlAttr(g) + "</saml:AttributeValue>";
         }
     }
-    return template.replace(re, values);
+    // Function replacer: the values string may contain a group name with $&/$`/$'
+    // which String.replace would otherwise treat as special replacement patterns
+    // and splice surrounding markup back into the (about-to-be-signed) assertion.
+    return template.replace(re, () => values);
 };
 
 
@@ -181,9 +184,13 @@ const makeLoginResponseRenderer = (opts) => {
             tvalue.InResponseTo = (opts.requestInfo && opts.requestInfo.extract && opts.requestInfo.extract.request && opts.requestInfo.extract.request.id) || "";
         }
         for (const k of Object.keys(tvalue)) {
-            work = work.replace(new RegExp("\\{" + k + "\\}", "g"), escapeXmlAttr(tvalue[k]));
+            // Function replacer so a value containing $&/$`/$'/$n is inserted
+            // literally (escapeXmlAttr does not neutralize $) and cannot inject
+            // markup into the signed assertion.
+            const escaped = escapeXmlAttr(tvalue[k]);
+            work = work.replace(new RegExp("\\{" + k + "\\}", "g"), () => escaped);
         }
-        work = work.replace(/\{AuthnStatement\}/g, buildAuthnStatement(nowIso, sessionIndex));
+        work = work.replace(/\{AuthnStatement\}/g, () => buildAuthnStatement(nowIso, sessionIndex));
         return { id: respId, context: work };
     };
 };
@@ -204,7 +211,10 @@ const makeLogoutResponseRenderer = (opts) => {
         };
         let work = template;
         for (const k of Object.keys(tvalue)) {
-            work = work.replace(new RegExp("\\{" + k + "\\}", "g"), escapeXmlAttr(tvalue[k]));
+            // Function replacer: see makeLoginResponseRenderer -- $-patterns in a
+            // value must not be interpreted as special replacement sequences.
+            const escaped = escapeXmlAttr(tvalue[k]);
+            work = work.replace(new RegExp("\\{" + k + "\\}", "g"), () => escaped);
         }
         return { id: respId, context: work };
     };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "saltcorn-idp",
-  "version": "0.0.5",
+  "version": "0.0.7",
   "description": "Saltcorn plugin: turns Saltcorn into an SSO Identity Provider (OIDC/OAuth2, LDAP with groups, and SAML 2.0). Per-tenant asymmetric signing keys sealed at rest; multi-tenant. See VENDORING.md for the dependency-ownership/security posture.",
   "main": "index.js",
   "scripts": {