fix(security): restrict local path extraction in media parser to prevent LFI (#4880)
* Media: restrict local path extraction to prevent LFI * Lint: remove unused variable hasValidMediaOnLine
This commit is contained in:
@@ -9,21 +9,33 @@ describe("splitMediaFromOutput", () => {
|
||||
expect(result.text).toBe("Hello world");
|
||||
});
|
||||
|
||||
it("captures media paths with spaces", () => {
|
||||
it("rejects absolute media paths to prevent LFI", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:/Users/pete/My File.png");
|
||||
expect(result.mediaUrls).toEqual(["/Users/pete/My File.png"]);
|
||||
expect(result.text).toBe("");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:/Users/pete/My File.png");
|
||||
});
|
||||
|
||||
it("captures quoted media paths with spaces", () => {
|
||||
it("rejects quoted absolute media paths to prevent LFI", () => {
|
||||
const result = splitMediaFromOutput('MEDIA:"/Users/pete/My File.png"');
|
||||
expect(result.mediaUrls).toEqual(["/Users/pete/My File.png"]);
|
||||
expect(result.text).toBe("");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe('MEDIA:"/Users/pete/My File.png"');
|
||||
});
|
||||
|
||||
it("captures tilde media paths with spaces", () => {
|
||||
it("rejects tilde media paths to prevent LFI", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:~/Pictures/My File.png");
|
||||
expect(result.mediaUrls).toEqual(["~/Pictures/My File.png"]);
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:~/Pictures/My File.png");
|
||||
});
|
||||
|
||||
it("rejects directory traversal media paths to prevent LFI", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:../../etc/passwd");
|
||||
expect(result.mediaUrls).toBeUndefined();
|
||||
expect(result.text).toBe("MEDIA:../../etc/passwd");
|
||||
});
|
||||
|
||||
it("captures safe relative media paths", () => {
|
||||
const result = splitMediaFromOutput("MEDIA:./screenshots/image.png");
|
||||
expect(result.mediaUrls).toEqual(["./screenshots/image.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
|
||||
@@ -43,8 +55,8 @@ describe("splitMediaFromOutput", () => {
|
||||
});
|
||||
|
||||
it("parses MEDIA tags with leading whitespace", () => {
|
||||
const result = splitMediaFromOutput(" MEDIA:/tmp/screenshot.png");
|
||||
expect(result.mediaUrls).toEqual(["/tmp/screenshot.png"]);
|
||||
const result = splitMediaFromOutput(" MEDIA:./screenshot.png");
|
||||
expect(result.mediaUrls).toEqual(["./screenshot.png"]);
|
||||
expect(result.text).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -19,11 +19,9 @@ function isValidMedia(candidate: string, opts?: { allowSpaces?: boolean }) {
|
||||
if (candidate.length > 4096) return false;
|
||||
if (!opts?.allowSpaces && /\s/.test(candidate)) return false;
|
||||
if (/^https?:\/\//i.test(candidate)) return true;
|
||||
if (candidate.startsWith("/")) return true;
|
||||
if (candidate.startsWith("./")) return true;
|
||||
if (candidate.startsWith("../")) return true;
|
||||
if (candidate.startsWith("~")) return true;
|
||||
return false;
|
||||
|
||||
// Local paths: only allow safe relative paths starting with ./ that do not traverse upwards.
|
||||
return candidate.startsWith("./") && !candidate.includes("..");
|
||||
}
|
||||
|
||||
function unwrapQuoted(value: string): string | undefined {
|
||||
@@ -85,10 +83,8 @@ export function splitMediaFromOutput(raw: string): {
|
||||
continue;
|
||||
}
|
||||
|
||||
foundMediaToken = true;
|
||||
const pieces: string[] = [];
|
||||
let cursor = 0;
|
||||
let hasValidMedia = false;
|
||||
|
||||
for (const match of matches) {
|
||||
const start = match.index ?? 0;
|
||||
@@ -101,11 +97,13 @@ export function splitMediaFromOutput(raw: string): {
|
||||
const mediaStartIndex = media.length;
|
||||
let validCount = 0;
|
||||
const invalidParts: string[] = [];
|
||||
let hasValidMedia = false;
|
||||
for (const part of parts) {
|
||||
const candidate = normalizeMediaSource(cleanCandidate(part));
|
||||
if (isValidMedia(candidate, unwrapped ? { allowSpaces: true } : undefined)) {
|
||||
media.push(candidate);
|
||||
hasValidMedia = true;
|
||||
foundMediaToken = true;
|
||||
validCount += 1;
|
||||
} else {
|
||||
invalidParts.push(part);
|
||||
@@ -130,6 +128,7 @@ export function splitMediaFromOutput(raw: string): {
|
||||
if (isValidMedia(fallback, { allowSpaces: true })) {
|
||||
media.splice(mediaStartIndex, media.length - mediaStartIndex, fallback);
|
||||
hasValidMedia = true;
|
||||
foundMediaToken = true;
|
||||
validCount = 1;
|
||||
invalidParts.length = 0;
|
||||
}
|
||||
@@ -140,12 +139,18 @@ export function splitMediaFromOutput(raw: string): {
|
||||
if (isValidMedia(fallback, { allowSpaces: true })) {
|
||||
media.push(fallback);
|
||||
hasValidMedia = true;
|
||||
foundMediaToken = true;
|
||||
invalidParts.length = 0;
|
||||
}
|
||||
}
|
||||
|
||||
if (hasValidMedia && invalidParts.length > 0) {
|
||||
pieces.push(invalidParts.join(" "));
|
||||
if (hasValidMedia) {
|
||||
if (invalidParts.length > 0) {
|
||||
pieces.push(invalidParts.join(" "));
|
||||
}
|
||||
} else {
|
||||
// If no valid media was found in this match, keep the original token text.
|
||||
pieces.push(match[0]);
|
||||
}
|
||||
|
||||
cursor = start + match[0].length;
|
||||
|
||||
Reference in New Issue
Block a user