Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions review-enrichment/src/analyzers/iac-misconfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@ const NO_NEW_PRIVILEGES_OFF_RE = /\bno-new-privileges[=:]\s*["']?false\b/i;
const DOCKER_SOCKET_MOUNT_RE =
/\/var\/run\/docker\.sock:|\bsource\s*:\s*["']?\/var\/run\/docker\.sock\b/;

// HTTP security-header misconfigurations (nginx/Apache/Caddy conf, Helm ingress annotations, netlify.toml
// headers, …). Each keys on a value that is the weakening itself, so there is no safe-value form of the same
// line. `unsafe-inline`/`unsafe-eval` are Content-Security-Policy-only keywords. The HSTS and Referrer-Policy
// rules require their own header token on the line, so `Cache-Control: max-age=0` (a normal caching directive)
// is NOT matched by the HSTS rule.
const CSP_UNSAFE_INLINE_RE = /\bunsafe-inline\b/i;
const CSP_UNSAFE_EVAL_RE = /\bunsafe-eval\b/i;
const HSTS_DISABLED_RE = /\bStrict-Transport-Security\b[^\n]*\bmax-age\s*=\s*0\b/i;
const REFERRER_UNSAFE_URL_RE = /\bReferrer-Policy\b[^\n]*\bunsafe-url\b/i;
const COOKIE_NOT_HTTPONLY_RE = /\bhttp[_-]?only\b[\s"'=:,-]*false\b/i;

function* patchLines(patch: string): Generator<string> {
let start = 0;
for (let i = 0; i <= patch.length; i++) {
Expand Down Expand Up @@ -515,6 +526,36 @@ export function scanPatchForIacMisconfig(
) {
return findings;
}
if (
CSP_UNSAFE_INLINE_RE.test(body) &&
pushFinding(findings, seen, path, newLine, "csp-unsafe-inline", maxFindings)
) {
return findings;
}
if (
CSP_UNSAFE_EVAL_RE.test(body) &&
pushFinding(findings, seen, path, newLine, "csp-unsafe-eval", maxFindings)
) {
return findings;
}
if (
HSTS_DISABLED_RE.test(body) &&
pushFinding(findings, seen, path, newLine, "hsts-disabled", maxFindings)
) {
return findings;
}
if (
REFERRER_UNSAFE_URL_RE.test(body) &&
pushFinding(findings, seen, path, newLine, "referrer-policy-leak", maxFindings)
) {
return findings;
}
if (
COOKIE_NOT_HTTPONLY_RE.test(body) &&
pushFinding(findings, seen, path, newLine, "cookie-not-httponly", maxFindings)
) {
return findings;
}

newLine++;
}
Expand Down
10 changes: 10 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,16 @@ export function renderBrief(
return "disables the `no-new-privileges` protection, allowing setuid binaries to escalate privileges";
case "docker-socket-mount":
return "mounts the host Docker socket (`/var/run/docker.sock`) into the container — this grants host-level control";
case "csp-unsafe-inline":
return "uses `'unsafe-inline'` in a Content-Security-Policy, permitting inline scripts/styles and weakening XSS protection";
case "csp-unsafe-eval":
return "uses `'unsafe-eval'` in a Content-Security-Policy, permitting `eval()` and weakening XSS protection";
case "hsts-disabled":
return "disables HSTS with `Strict-Transport-Security` `max-age=0`, so browsers stop enforcing HTTPS for the host";
case "referrer-policy-leak":
return "sets `Referrer-Policy: unsafe-url`, leaking the full URL (path and query) to cross-origin destinations";
case "cookie-not-httponly":
return "sets `httpOnly: false` on a cookie, exposing it to JavaScript so an XSS can read it";
}
};

Expand Down
7 changes: 6 additions & 1 deletion review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,12 @@ export interface IacMisconfigFinding {
| "ipc-host"
| "cap-add-all"
| "no-new-privileges-off"
| "docker-socket-mount";
| "docker-socket-mount"
| "csp-unsafe-inline"
| "csp-unsafe-eval"
| "hsts-disabled"
| "referrer-policy-leak"
| "cookie-not-httponly";
}

/** A newly-added dependency whose install compiles native code (npm node-gyp addon) or has no prebuilt wheel
Expand Down
40 changes: 40 additions & 0 deletions review-enrichment/test/iac-misconfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,3 +444,43 @@ test("scanPatchForIacMisconfig does not flag the secure counterpart of each cont
);
}
});

test("scanPatchForIacMisconfig flags insecure HTTP security-header settings", () => {
// Each matched value is the weakening itself, so there is no safe-value form of the same line.
const cases = [
["+ add_header Content-Security-Policy \"script-src 'unsafe-inline'\";", "csp-unsafe-inline"],
["+ Content-Security-Policy: script-src 'unsafe-eval'", "csp-unsafe-eval"],
["+ add_header Strict-Transport-Security \"max-age=0\";", "hsts-disabled"],
["+ add_header Referrer-Policy \"unsafe-url\";", "referrer-policy-leak"],
["+ httpOnly: false", "cookie-not-httponly"],
];
for (const [added, kind] of cases) {
const findings = scanPatchForIacMisconfig(
"nginx.conf",
["@@ -1,0 +1,1 @@", added].join("\n"),
);
assert.deepEqual(
findings,
[{ file: "nginx.conf", line: 1, kind }],
`${kind}: expected exactly one finding of that kind, got ${JSON.stringify(findings)}`,
);
}
});

test("scanPatchForIacMisconfig does not flag secure HTTP header values (incl. Cache-Control max-age=0)", () => {
const safe = [
"+ Content-Security-Policy: script-src 'self'",
"+ add_header Strict-Transport-Security \"max-age=31536000; includeSubDomains\";",
// Cache-Control max-age=0 is a NORMAL caching directive and must NOT fire the HSTS rule.
"+ add_header Cache-Control \"max-age=0\";",
"+ add_header Referrer-Policy \"strict-origin-when-cross-origin\";",
"+ httpOnly: true",
];
for (const added of safe) {
assert.deepEqual(
scanPatchForIacMisconfig("nginx.conf", ["@@ -1,0 +1,1 @@", added].join("\n")),
[],
`should not flag: ${added.trim()}`,
);
}
});
Loading