Skip to content

Commit 0c29b1e

Browse files
committed
fix: Arbitrary File Write, close #14
1 parent 9ac8803 commit 0c29b1e

File tree

6 files changed

+183
-12
lines changed

6 files changed

+183
-12
lines changed

src/fs.ts

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,21 @@ export async function getFileEntry(target: string): Promise<FileEntry> {
6565
};
6666
}
6767

68-
export async function ensureFolder(folder: string): Promise<void> {
68+
export async function ensureFolder(folder: string): Promise<{
69+
isDirectory: boolean,
70+
isSymbolicLink: boolean,
71+
realpath?: string,
72+
}> {
6973
// stop at root
7074
if (folder === path.dirname(folder)) {
71-
return Promise.resolve();
75+
return Promise.resolve({
76+
isDirectory: true,
77+
isSymbolicLink: false
78+
});
7279
}
7380
try {
74-
await mkdir(folder);
81+
const result = await mkdir(folder);
82+
return result;
7583
} catch (error) {
7684
// ENOENT: a parent folder does not exist yet, continue
7785
// to create the parent folder and then try again.
@@ -125,9 +133,17 @@ export async function rimraf(target: string): Promise<void> {
125133
}
126134
}
127135

128-
async function mkdir(folder: string): Promise<void> {
136+
async function mkdir(folder: string): Promise<{
137+
isDirectory: boolean,
138+
isSymbolicLink: boolean,
139+
realpath?: string,
140+
}> {
129141
try {
130142
await fs.mkdir(folder, 0o777);
143+
return {
144+
isDirectory: true,
145+
isSymbolicLink: false,
146+
};
131147
} catch (error) {
132148
// ENOENT: a parent folder does not exist yet or folder name is invalid.
133149
if (error.code === "ENOENT") {
@@ -136,14 +152,80 @@ async function mkdir(folder: string): Promise<void> {
136152
// Any other error: check if folder exists and
137153
// return normally in that case if its a folder
138154
try {
139-
const fileStat = await fs.stat(folder);
140-
if (!fileStat.isDirectory()) {
141-
return Promise.reject(new Error(`"${folder}" exists and is not a directory.`));
155+
const fileStat = await fs.lstat(folder);
156+
if (fileStat.isSymbolicLink()) {
157+
const realFilePath = await realpath(folder);
158+
const realFileStat = await fs.lstat(realFilePath);
159+
if (!realFileStat.isDirectory()) {
160+
return Promise.reject(new Error(`"${folder}" exists and is not a directory.`));
161+
}
162+
return {
163+
isDirectory: false,
164+
isSymbolicLink: true,
165+
realpath: realFilePath,
166+
}
167+
} else {
168+
if (!fileStat.isDirectory()) {
169+
return Promise.reject(new Error(`"${folder}" exists and is not a directory.`));
170+
}
171+
return {
172+
isDirectory: true,
173+
isSymbolicLink: false,
174+
}
142175
}
143176
} catch (statError) {
144177
throw error; // rethrow original error
145178
}
146179
}
180+
181+
// // Any other error: check if folder exists and
182+
// // return normally in that case if its a folder
183+
// try {
184+
// if (folder.includes("dirlink")) {
185+
// console.log("tttttt11111", folder);
186+
// }
187+
// const fileStat = await fs.lstat(folder);
188+
// if (folder.includes("dirlink")) {
189+
// console.log("tttttt22222", fileStat);
190+
// }
191+
// if (fileStat.isSymbolicLink()) {
192+
// console.log("kkkkkkkkk", fileStat);
193+
// const realFilePath = await realpath(folder);
194+
// const realFileStat = await fs.lstat(realFilePath);
195+
// if (!realFileStat.isDirectory()) {
196+
// return Promise.reject(new Error(`"${folder}" exists and is not a directory.`));
197+
// }
198+
// return {
199+
// isDirectory: false,
200+
// isSymbolicLink: true,
201+
// realpath: realFilePath,
202+
// }
203+
// } else {
204+
// if (!fileStat.isDirectory()) {
205+
// return Promise.reject(new Error(`"${folder}" exists and is not a directory.`));
206+
// }
207+
// return {
208+
// isDirectory: true,
209+
// isSymbolicLink: false,
210+
// }
211+
// }
212+
// } catch (statError) {
213+
// if (folder.includes("dirlink")) {
214+
// console.log("yyyyy", statError);
215+
// }
216+
// // ignore
217+
// }
218+
// if (folder.includes("dirlink")) {
219+
// console.log("kkkkkkkkk22222", folder);
220+
// }
221+
// await fs.mkdir(folder, { recursive: true, mode: 0o777 });
222+
// if (folder.includes("dirlink")) {
223+
// console.log("kkkkkkkkk3333333", folder);
224+
// }
225+
// return {
226+
// isDirectory: true,
227+
// isSymbolicLink: false,
228+
// };
147229
}
148230

149231
// "A"

src/unzip.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ interface IEntryContext {
9797
* The name of the symlink file that has been processed.
9898
*/
9999
readonly symlinkFileNames: string[];
100+
/**
101+
* The name of the symlink folder that has been processed.
102+
*/
103+
readonly symlinkFolders: { folder: string, realpath: string }[];
100104
getFilePath(): string;
101105
/**
102106
* Whether the specified path is outside the target folder
@@ -110,6 +114,7 @@ class EntryContext implements IEntryContext {
110114
private _realTargetFolder: string,
111115
private symlinkAsFileOnWindows: boolean) {
112116
this._symlinkFileNames = [];
117+
this._symlinkFolders = [];
113118
}
114119
private _decodeEntryFileName: string;
115120
public get decodeEntryFileName(): string {
@@ -128,19 +133,32 @@ class EntryContext implements IEntryContext {
128133
public get symlinkFileNames(): string[] {
129134
return this._symlinkFileNames;
130135
}
136+
private _symlinkFolders: { folder: string, realpath: string }[];
137+
public get symlinkFolders(): { folder: string, realpath: string }[] {
138+
return this._symlinkFolders;
139+
}
131140

132141
public getFilePath(): string {
133142
return path.join(this.targetFolder, this.decodeEntryFileName);
134143
}
135144

136145
public async isOutsideTargetFolder(tpath: string): Promise<boolean> {
137-
if (this.symlinkFileNames.length === 0) {
146+
if (this.symlinkFileNames.length === 0 &&
147+
this.symlinkFolders.length === 0
148+
) {
138149
return false;
139150
}
140151
if (process.platform === "win32" &&
141152
this.symlinkAsFileOnWindows) {
142153
return false;
143154
}
155+
for (const { folder, realpath } of this.symlinkFolders) {
156+
if (tpath.includes(folder)) {
157+
if (realpath.indexOf(this.realTargetFolder) !== 0) {
158+
return true;
159+
}
160+
}
161+
}
144162
for (const fileName of this.symlinkFileNames) {
145163
if (tpath.includes(fileName)) {
146164
const realFilePath = await exfs.realpath(tpath);
@@ -318,7 +336,13 @@ export class Unzip extends Cancelable {
318336
private async extractEntry(zfile: yauzl.ZipFile, entry: yauzl.Entry, entryContext: IEntryContext, token: CancellationToken): Promise<void> {
319337
const filePath = entryContext.getFilePath();
320338
const fileDir = path.dirname(filePath);
321-
await exfs.ensureFolder(fileDir);
339+
const folderStat = await exfs.ensureFolder(fileDir);
340+
if (folderStat.isSymbolicLink) {
341+
entryContext.symlinkFolders.push({
342+
folder: fileDir,
343+
realpath: folderStat.realpath!,
344+
})
345+
}
322346
const outside = await entryContext.isOutsideTargetFolder(fileDir);
323347
if (outside) {
324348
const error = new Error(`Refuse to write file outside "${entryContext.targetFolder}", file: "${filePath}"`);

test/src/unzipArbitraryFileWrite.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,84 @@
11
import * as zl from "../../dist";
2+
import * as fs from "../../dist/fs";
23
import * as path from "path";
34
import * as assert from "assert";
45

56
describe("unzip", () => {
6-
it("extract a zip file that attempt to write file outside ouput folder", async () => {
7+
it("extract a zip file that attempt to write file outside output folder", async () => {
78
try {
89
await zl.extract(path.join(__dirname, "../unzipResources/arbitrary_file_write.zip"), path.join(__dirname, "../unzips/arbitrary_file_write"), {
910
overwrite: true,
1011
symlinkAsFileOnWindows: false
1112
});
12-
assert.fail("extract a zip file that attempt to write file outside ouput folder");
13+
assert.fail("extract a zip file that attempt to write file outside output folder");
1314
} catch (error) {
1415
if (process.platform === "win32" &&
1516
error.code === "EPERM") {
16-
assert.ok(true, "code EPERM as expected");
17+
console.warn("Please run this test with administator.");
18+
assert.ok(true, "Please run this test with administator.");
19+
} else if (error.name === "AFWRITE") {
20+
assert.ok(true, "name AFWRITE as expected");
21+
} else {
22+
assert.fail(error);
23+
}
24+
}
25+
});
26+
it("extract a zip file that attempt to write file to symlink folder which is outside output folder", async () => {
27+
try {
28+
await fs.rimraf(path.join(__dirname, "../unzips/arbitrary_write/output"));
29+
await fs.rimraf(path.join(__dirname, "../unzips/arbitrary_write/tmp"));
30+
await fs.ensureFolder(path.join(__dirname, "../unzips/arbitrary_write/tmp"));
31+
await zl.extract(path.join(__dirname, "../unzipResources/arbitrary_write/output1.zip"), path.join(__dirname, "../unzips/arbitrary_write/output"), {
32+
overwrite: false,
33+
symlinkAsFileOnWindows: false
34+
});
35+
} catch (error) {
36+
if (process.platform === "win32" &&
37+
error.code === "EPERM") {
38+
console.warn("Please run this test with administator.");
39+
assert.ok(true, "Please run this test with administator.");
40+
} else if (error.name === "AFWRITE") {
41+
assert.ok(true, "name AFWRITE as expected");
42+
} else {
43+
assert.fail(error);
44+
}
45+
}
46+
47+
try {
48+
await zl.extract(path.join(__dirname, "../unzipResources/arbitrary_write/output2.zip"), path.join(__dirname, "../unzips/arbitrary_write/output"), {
49+
overwrite: false,
50+
symlinkAsFileOnWindows: false
51+
});
52+
if (process.platform === "win32") {
53+
assert.ok(true, "Please run this test with administator.");
54+
} else {
55+
assert.fail("extract a zip file that attempt to write file to symlink folder which is outside output folder");
56+
}
57+
} catch (error) {
58+
if (process.platform === "win32" &&
59+
error.code === "EPERM") {
60+
assert.ok(true, "Please run this test with administator.");
61+
} else if (error.name === "AFWRITE") {
62+
assert.ok(true, "name AFWRITE as expected");
63+
} else {
64+
assert.fail(error);
65+
}
66+
}
67+
68+
try {
69+
await zl.extract(path.join(__dirname, "../unzipResources/arbitrary_write/output3.zip"), path.join(__dirname, "../unzips/arbitrary_write/output"), {
70+
overwrite: false,
71+
symlinkAsFileOnWindows: false
72+
});
73+
if (process.platform === "win32") {
74+
assert.ok(true, "Please run this test with administator.");
75+
} else {
76+
assert.fail("extract a zip file that attempt to write file to symlink folder which is outside output folder");
77+
}
78+
} catch (error) {
79+
if (process.platform === "win32" &&
80+
error.code === "EPERM") {
81+
assert.ok(true, "Please run this test with administator.");
1782
} else if (error.name === "AFWRITE") {
1883
assert.ok(true, "name AFWRITE as expected");
1984
} else {
119 Bytes
Binary file not shown.
148 Bytes
Binary file not shown.
145 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)