Skip to content

Commit c46dbda

Browse files
authored
fix: argument Injection via repositoryUrl in package.json (#4245)
Co-Authored-By: Abhijith S abh78i00@gmail.com
1 parent 7abb562 commit c46dbda

6 files changed

Lines changed: 76 additions & 11 deletions

File tree

index.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,9 @@ declare module "semantic-release" {
627627
* Any valid git url format is supported (see
628628
* [git protocols](https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols))
629629
*
630+
* The value must not begin with `-`, as it would otherwise be interpreted
631+
* by `git` as a command-line option rather than a URL.
632+
*
630633
* Default: `repository` property in `package.json`, or git origin url.
631634
*/
632635
repositoryUrl?: string | undefined;

lib/definitions/errors.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,21 @@ Please make sure to add the \`repositoryUrl\` to the [semantic-release configura
4242
};
4343
}
4444

45+
export function EINVALIDREPOURL({ repositoryUrl }) {
46+
return {
47+
message: "The `repositoryUrl` option is not a valid git URL.",
48+
details: `The [repositoryUrl option](${linkify(
49+
"usage/configuration#repositoryurl"
50+
)}) must be a valid git URL and cannot begin with \`-\`, as \`git\` would otherwise interpret it as a command-line option rather than a URL.
51+
52+
The invalid \`repositoryUrl\` is \`${repositoryUrl}\`.
53+
54+
Please verify the \`repository\` property in your \`package.json\` and the [repositoryUrl option](${linkify(
55+
"usage/configuration#repositoryurl"
56+
)}) in your [semantic-release configuration](${linkify("usage/configuration#configuration")}).`,
57+
};
58+
}
59+
4560
export function EGITNOPERMISSION({ options: { repositoryUrl }, branch: { name } }) {
4661
return {
4762
message: "Cannot push to the Git repository.",

lib/git.js

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export async function getCommits(from, to, execaOptions) {
6767
* @throws {Error} If the `git` command fails.
6868
*/
6969
export async function getBranches(repositoryUrl, execaOptions) {
70-
return (await execa("git", ["ls-remote", "--heads", repositoryUrl], execaOptions)).stdout
70+
return (await execa("git", ["ls-remote", "--heads", "--", repositoryUrl], execaOptions)).stdout
7171
.split("\n")
7272
.filter(Boolean)
7373
.map((branch) => branch.match(/^.+refs\/heads\/(?<branch>.+)$/)[1]);
@@ -115,8 +115,8 @@ export async function fetch(repositoryUrl, branch, ciBranch, execaOptions) {
115115
"--unshallow",
116116
"--tags",
117117
...(branch === ciBranch && !isDetachedHead
118-
? [repositoryUrl]
119-
: ["--update-head-ok", repositoryUrl, `+refs/heads/${branch}:refs/heads/${branch}`]),
118+
? ["--", repositoryUrl]
119+
: ["--update-head-ok", "--", repositoryUrl, `+refs/heads/${branch}:refs/heads/${branch}`]),
120120
],
121121
execaOptions
122122
);
@@ -127,8 +127,8 @@ export async function fetch(repositoryUrl, branch, ciBranch, execaOptions) {
127127
"fetch",
128128
"--tags",
129129
...(branch === ciBranch && !isDetachedHead
130-
? [repositoryUrl]
131-
: ["--update-head-ok", repositoryUrl, `+refs/heads/${branch}:refs/heads/${branch}`]),
130+
? ["--", repositoryUrl]
131+
: ["--update-head-ok", "--", repositoryUrl, `+refs/heads/${branch}:refs/heads/${branch}`]),
132132
],
133133
execaOptions
134134
);
@@ -143,9 +143,9 @@ export async function fetch(repositoryUrl, branch, ciBranch, execaOptions) {
143143
*/
144144
export async function fetchNotes(repositoryUrl, execaOptions) {
145145
try {
146-
await execa("git", ["fetch", "--unshallow", repositoryUrl, `+refs/notes/*:refs/notes/*`], execaOptions);
146+
await execa("git", ["fetch", "--unshallow", "--", repositoryUrl, `+refs/notes/*:refs/notes/*`], execaOptions);
147147
} catch {
148-
await execa("git", ["fetch", repositoryUrl, `+refs/notes/*:refs/notes/*`], {
148+
await execa("git", ["fetch", "--", repositoryUrl, `+refs/notes/*:refs/notes/*`], {
149149
...execaOptions,
150150
reject: false,
151151
});
@@ -204,7 +204,7 @@ export async function isGitRepo(execaOptions) {
204204
*/
205205
export async function verifyAuth(repositoryUrl, branch, execaOptions) {
206206
try {
207-
await execa("git", ["push", "--dry-run", "--no-verify", repositoryUrl, `HEAD:${branch}`], execaOptions);
207+
await execa("git", ["push", "--dry-run", "--no-verify", "--", repositoryUrl, `HEAD:${branch}`], execaOptions);
208208
} catch (error) {
209209
debug(error);
210210
throw error;
@@ -233,7 +233,7 @@ export async function tag(tagName, ref, execaOptions) {
233233
* @throws {Error} if the push failed.
234234
*/
235235
export async function push(repositoryUrl, execaOptions) {
236-
await execa("git", ["push", "--tags", repositoryUrl], execaOptions);
236+
await execa("git", ["push", "--tags", "--", repositoryUrl], execaOptions);
237237
}
238238

239239
/**
@@ -245,7 +245,7 @@ export async function push(repositoryUrl, execaOptions) {
245245
* @throws {Error} if the push failed.
246246
*/
247247
export async function pushNotes(repositoryUrl, ref, execaOptions) {
248-
await execa("git", ["push", repositoryUrl, `refs/notes/${GIT_NOTE_REF}-${ref}`], execaOptions);
248+
await execa("git", ["push", "--", repositoryUrl, `refs/notes/${GIT_NOTE_REF}-${ref}`], execaOptions);
249249
}
250250

251251
/**
@@ -292,7 +292,9 @@ export async function verifyBranchName(branch, execaOptions) {
292292
export async function isBranchUpToDate(repositoryUrl, branch, execaOptions) {
293293
return (
294294
(await getGitHead(execaOptions)) ===
295-
(await execa("git", ["ls-remote", "--heads", repositoryUrl, branch], execaOptions)).stdout.match(/^(?<ref>\w+)?/)[1]
295+
(await execa("git", ["ls-remote", "--heads", "--", repositoryUrl, branch], execaOptions)).stdout.match(
296+
/^(?<ref>\w+)?/
297+
)[1]
296298
);
297299
}
298300

lib/verify.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ export default async (context) => {
1515
errors.push(getError("ENOGITREPO", { cwd }));
1616
} else if (!repositoryUrl) {
1717
errors.push(getError("ENOREPOURL"));
18+
} else if (repositoryUrl.startsWith("-")) {
19+
// A `repositoryUrl` beginning with `-` would be interpreted by `git` as a
20+
// command-line option rather than a URL, enabling argument injection.
21+
errors.push(getError("EINVALIDREPOURL", { repositoryUrl }));
1822
}
1923

2024
// Verify that compiling the `tagFormat` produce a valid Git tag

test/git.test.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { existsSync } from "node:fs";
2+
import path from "node:path";
13
import test from "ava";
24
import { temporaryDirectory } from "tempy";
35
import {
@@ -423,3 +425,30 @@ test("Fetch all notes on a detached head repository", async (t) => {
423425

424426
t.is(await gitGetNote(commit.hash, { cwd }), '{"note":"note"}');
425427
});
428+
429+
test("Does not execute a `repositoryUrl` injected as an `--upload-pack` git option", async (t) => {
430+
const { cwd } = await gitRepo();
431+
await gitCommits(["First"], { cwd });
432+
const marker = path.join(temporaryDirectory(), "upload-pack-rce");
433+
// A `repositoryUrl` beginning with `-` would be parsed by git as a
434+
// command-line option rather than a positional argument. `--upload-pack`
435+
// lets the injected value run an arbitrary binary when fetching refs.
436+
const repositoryUrl = `--upload-pack=touch ${marker}`;
437+
438+
await t.throwsAsync(isBranchUpToDate(repositoryUrl, "master", { cwd }));
439+
440+
t.false(existsSync(marker));
441+
});
442+
443+
test("Does not execute a `repositoryUrl` injected as a `--receive-pack` git option", async (t) => {
444+
const { cwd } = await gitRepo(true);
445+
await gitCommits(["First"], { cwd });
446+
const marker = path.join(temporaryDirectory(), "receive-pack-rce");
447+
// `--receive-pack` is the push-side equivalent: it lets the injected value
448+
// run an arbitrary binary when pushing to the (configured) remote.
449+
const repositoryUrl = `--receive-pack=touch ${marker}`;
450+
451+
await t.throwsAsync(push(repositoryUrl, { cwd }));
452+
453+
t.false(existsSync(marker));
454+
});

test/verify.test.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,18 @@ test("Throw a SemanticReleaseError for each invalid branch", async (t) => {
109109
t.truthy(errors[5].details);
110110
});
111111

112+
test('Throw a SemanticReleaseError if the "repositoryUrl" starts with "-"', async (t) => {
113+
const { cwd } = await gitRepo(true);
114+
const options = { repositoryUrl: "--upload-pack=/bin/sh", tagFormat: `v\${version}`, branches: [{ name: "master" }] };
115+
116+
const errors = [...(await t.throwsAsync(verify({ cwd, options }))).errors];
117+
118+
t.is(errors[0].name, "SemanticReleaseError");
119+
t.is(errors[0].code, "EINVALIDREPOURL");
120+
t.truthy(errors[0].message);
121+
t.truthy(errors[0].details);
122+
});
123+
112124
test('Return "true" if all verification pass', async (t) => {
113125
const { cwd, repositoryUrl } = await gitRepo(true);
114126
const options = { repositoryUrl, tagFormat: `v\${version}`, branches: [{ name: "master" }] };

0 commit comments

Comments
 (0)