mirror of
https://github.com/AikidoSec/safe-chain.git
synced 2026-05-26 12:10:49 +00:00
Some fixes
This commit is contained in:
parent
091e6ec5f8
commit
2bc6d249de
2 changed files with 121 additions and 8 deletions
|
|
@ -15,6 +15,8 @@ import { ui } from "../environment/userInteraction.js";
|
|||
*/
|
||||
function isParsable(pem) {
|
||||
if (!pem || typeof pem !== "string") return false;
|
||||
// Normalize Windows CRLF to LF to ensure consistent parsing
|
||||
pem = pem.replace(/\r\n/g, "\n").replace(/\r/g, "\n");
|
||||
const begin = "-----BEGIN CERTIFICATE-----";
|
||||
const end = "-----END CERTIFICATE-----";
|
||||
const blocks = [];
|
||||
|
|
@ -95,6 +97,15 @@ export function getCombinedCaBundlePath() {
|
|||
return cachedPath;
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize path
|
||||
* @param {string} p - Path to normalize
|
||||
* @returns {string}
|
||||
*/
|
||||
function normalizePathF(p) {
|
||||
return p.replace(/\\/g, "/");
|
||||
}
|
||||
|
||||
/**
|
||||
* Read and validate user certificate file
|
||||
* @param {string} certPath - Path to certificate file
|
||||
|
|
@ -102,32 +113,50 @@ export function getCombinedCaBundlePath() {
|
|||
*/
|
||||
function readUserCertificateFile(certPath) {
|
||||
try {
|
||||
// Perform security checks before reading
|
||||
// 1) Basic validation
|
||||
if (typeof certPath !== "string" || certPath.trim().length === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (certPath.includes("..") || certPath.includes("//") || certPath.includes("\\\\")) {
|
||||
// 2) Reject path traversal attempts (normalize backslashes first for Windows paths)
|
||||
const normalizedPath = normalizePathF(certPath);
|
||||
if (normalizedPath.includes("..")) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (!fs.existsSync(certPath)) {
|
||||
// 3) Check if file exists and is not a directory or symlink
|
||||
let stats;
|
||||
try {
|
||||
stats = fs.lstatSync(certPath);
|
||||
} catch {
|
||||
// File doesn't exist or can't be accessed
|
||||
return null;
|
||||
}
|
||||
|
||||
const stats = fs.lstatSync(certPath);
|
||||
if (!stats.isFile() || stats.isSymbolicLink()) {
|
||||
if (!stats.isFile()) {
|
||||
// Reject directories and symlinks
|
||||
return null;
|
||||
}
|
||||
|
||||
// 4) Read file content
|
||||
let content;
|
||||
try {
|
||||
content = fs.readFileSync(certPath, "utf8");
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
const content = fs.readFileSync(certPath, "utf8");
|
||||
if (!content || typeof content !== "string") {
|
||||
return null;
|
||||
}
|
||||
|
||||
// 6) Validate PEM format
|
||||
// 5) Validate PEM format
|
||||
if (!isParsable(content)) {
|
||||
return null;
|
||||
// Fallback: accept if it at least contains PEM delimiters
|
||||
// (covers edge cases with unusual formatting that X509Certificate might reject)
|
||||
if (!content.includes("-----BEGIN CERTIFICATE-----") || !content.includes("-----END CERTIFICATE-----")) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
return content;
|
||||
|
|
|
|||
|
|
@ -272,4 +272,88 @@ describe("certBundle.getCombinedCaBundlePathWithUserCerts", () => {
|
|||
const contents = fs.readFileSync(bundlePath, "utf8");
|
||||
assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA");
|
||||
});
|
||||
|
||||
it("accepts files with CRLF line endings (Windows-style)", async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
|
||||
const safeChainPath = path.join(tmpDir, "safechain.pem");
|
||||
fs.writeFileSync(safeChainPath, getValidCert(), "utf8");
|
||||
|
||||
// Create a real file with CRLF content to test Windows line ending support
|
||||
const userCertPath = path.join(tmpDir, "user-cert-crlf.pem");
|
||||
const crlfCert = getValidCert().replace(/\n/g, "\r\n");
|
||||
fs.writeFileSync(userCertPath, crlfCert, "utf8");
|
||||
|
||||
mock.module("./certUtils.js", {
|
||||
namedExports: {
|
||||
getCaCertPath: () => safeChainPath,
|
||||
},
|
||||
});
|
||||
|
||||
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js");
|
||||
const bundlePath = getCombinedCaBundlePathWithUserCerts(userCertPath);
|
||||
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
|
||||
const contents = fs.readFileSync(bundlePath, "utf8");
|
||||
const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length;
|
||||
assert.ok(certCount >= 2, "Bundle should contain Safe Chain and user certificates with CRLF");
|
||||
});
|
||||
|
||||
it("detects and handles Windows-style path syntax (drive letters and UNC)", async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
|
||||
const safeChainPath = path.join(tmpDir, "safechain.pem");
|
||||
fs.writeFileSync(safeChainPath, getValidCert(), "utf8");
|
||||
|
||||
mock.module("./certUtils.js", {
|
||||
namedExports: {
|
||||
getCaCertPath: () => safeChainPath,
|
||||
},
|
||||
});
|
||||
|
||||
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js");
|
||||
|
||||
// Test that Windows path syntax is recognized (even if files don't exist on macOS/Linux)
|
||||
// These should gracefully fail (return Safe Chain CA only) rather than crash
|
||||
const winPaths = [
|
||||
"C:\\temp\\cert.pem",
|
||||
"D:\\Users\\name\\certs\\ca.pem",
|
||||
"\\\\server\\share\\cert.pem"
|
||||
];
|
||||
|
||||
for (const winPath of winPaths) {
|
||||
const bundlePath = getCombinedCaBundlePathWithUserCerts(winPath);
|
||||
assert.ok(fs.existsSync(bundlePath), `Bundle should exist for ${winPath}`);
|
||||
const contents = fs.readFileSync(bundlePath, "utf8");
|
||||
assert.match(contents, /-----BEGIN CERTIFICATE-----/, "Should contain Safe Chain CA");
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects path traversal with Windows-style paths (C:\\temp\\..\\etc\\passwd)", async () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "certtest-"));
|
||||
const safeChainPath = path.join(tmpDir, "safechain.pem");
|
||||
fs.writeFileSync(safeChainPath, getValidCert(), "utf8");
|
||||
|
||||
mock.module("./certUtils.js", {
|
||||
namedExports: {
|
||||
getCaCertPath: () => safeChainPath,
|
||||
},
|
||||
});
|
||||
|
||||
const { getCombinedCaBundlePathWithUserCerts } = await import("./certBundle.js");
|
||||
|
||||
// Test various Windows-style traversal attempts
|
||||
const traversalPaths = [
|
||||
"C:\\temp\\..\\etc\\passwd",
|
||||
"D:\\Users\\..\\..\\Windows\\System32",
|
||||
"\\\\server\\share\\..\\admin",
|
||||
"../../../etc/passwd", // Unix-style for comparison
|
||||
];
|
||||
|
||||
for (const badPath of traversalPaths) {
|
||||
const bundlePath = getCombinedCaBundlePathWithUserCerts(badPath);
|
||||
assert.ok(fs.existsSync(bundlePath), "Bundle path should exist");
|
||||
const contents = fs.readFileSync(bundlePath, "utf8");
|
||||
// Only Safe Chain CA should be present (user cert rejected due to traversal)
|
||||
const certCount = (contents.match(/-----BEGIN CERTIFICATE-----/g) || []).length;
|
||||
assert.strictEqual(certCount, 1, `Traversal path ${badPath} should be rejected; only Safe Chain CA included`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue