Unsafe operations plugin extended to prevent the use of potential att…

…ack vectors `--upload-pack` or `--receive-pack` (or the shorthand version `-u` in `git clone`) without explicitly opting in with the `allowUnsafePack` option.

packages/test-utils/src/expectations.ts

@@ -17,7 +17,12 @@ export function assertGitError(
  errorConstructor: any = GitError
 ) {
  expect(errorInstance).toBeInstanceOf(errorConstructor);
- expect(errorInstance).toHaveProperty('message', expect.stringMatching(message));
+ expect(errorInstance).toHaveProperty(
+ 'message',
+ typeof message === 'string'
+ ? expect.stringContaining(message)
+ : expect.stringMatching(message)
+ );
 }
 
 export function assertGitResponseError(errorInstance: Error | unknown, git: any, equality?: any) {

simple-git/src/lib/plugins/block-unsafe-operations-plugin.ts

@@ -23,16 +23,36 @@ function preventProtocolOverride(arg: string, next: string) {
  );
 }
 
+function preventUploadPack(arg: string, method: string) {
+ if (/^\s*--(upload|receive)-pack/.test(arg)) {
+ throw new GitPluginError(
+ undefined,
+ 'unsafe',
+ `Use of --upload-pack or --receive-pack is not permitted without enabling allowUnsafePack`
+ );
+ }
+
+ if (method === 'clone' && /^\s*-u\b/.test(arg)) {
+ throw new GitPluginError(
+ undefined,
+ 'unsafe',
+ `Use of clone with option -u is not permitted without enabling allowUnsafePack`
+ );
+ }
+}
+
 export function blockUnsafeOperationsPlugin({
  allowUnsafeProtocolOverride = false,
+ allowUnsafePack = false,
 }: SimpleGitPluginConfig['unsafe'] = {}): SimpleGitPlugin<'spawn.args'> {
  return {
  type: 'spawn.args',
- action(args, _context) {
+ action(args, context) {
  args.forEach((current, index) => {
  const next = index < args.length ? args[index + 1] : '';
 
  allowUnsafeProtocolOverride || preventProtocolOverride(current, next);
+ allowUnsafePack || preventUploadPack(current, context.method);
  });
 
  return args;

simple-git/src/lib/types/index.ts

@@ -119,10 +119,17 @@ export interface SimpleGitPluginConfig {
  *
  * Enable this override to use the `ext::` protocol (see examples on
  * [git-scm.com](https://git-scm.com/docs/git-remote-ext#_examples)).
- *
- * See documentation for use in
  */
  allowUnsafeProtocolOverride?: boolean;
+
+ /**
+ * Given the possibility of using `--upload-pack` and `--receive-pack` as
+ * attack vectors, the use of these in any command (or the shorthand
+ * `-u` option in a `clone` operation) are blocked by default.
+ *
+ * Enable this override to permit the use of these arguments.
+ */
+ allowUnsafePack?: boolean;
  };
 }
 

simple-git/test/unit/clean.spec.ts

@@ -1,6 +1,7 @@
 import { SimpleGit } from 'typings';
 import {
  assertExecutedCommands,
+ assertGitError,
  assertNoExecutedTasks,
  closeWithSuccess,
  newSimpleGit,
@@ -95,7 +96,7 @@ describe('clean', () => {
  it('handles configuration errors', async () => {
  const err = await git.clean('X').catch((e) => e);
 
- expectTheError(err).toBe(CONFIG_ERROR_MODE_REQUIRED);
+ assertGitError(err, CONFIG_ERROR_MODE_REQUIRED, TaskConfigurationError);
  });
  });
 
@@ -118,8 +119,8 @@ describe('clean', () => {
  'missing required n or f in mode',
  test((done) => {
  git.clean('x', function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_MODE_REQUIRED);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_MODE_REQUIRED, TaskConfigurationError);
+ assertNoExecutedTasks();
  done();
  });
  })
@@ -129,8 +130,8 @@ describe('clean', () => {
  'unknown options',
  test((done) => {
  git.clean('fa', function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_UNKNOWN_OPTION);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_UNKNOWN_OPTION, TaskConfigurationError);
+ assertNoExecutedTasks();
  done();
  });
  })
@@ -140,8 +141,8 @@ describe('clean', () => {
  'no args',
  test((done) => {
  git.clean(function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_MODE_REQUIRED);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_MODE_REQUIRED, TaskConfigurationError);
+ assertNoExecutedTasks();
  done();
  });
  })
@@ -199,8 +200,8 @@ describe('clean', () => {
  'prevents interactive mode - shorthand option',
  test((done) => {
  git.clean('f', ['-i'], function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_INTERACTIVE_MODE);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_INTERACTIVE_MODE, TaskConfigurationError);
+ assertNoExecutedTasks();
 
  done();
  });
@@ -211,8 +212,8 @@ describe('clean', () => {
  'prevents interactive mode - shorthand mode',
  test((done) => {
  git.clean('fi', function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_INTERACTIVE_MODE);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_INTERACTIVE_MODE, TaskConfigurationError);
+ assertNoExecutedTasks();
 
  done();
  });
@@ -223,28 +224,15 @@ describe('clean', () => {
  'prevents interactive mode - longhand option',
  test((done) => {
  git.clean('f', ['--interactive'], function (err: null | Error) {
- expectTheError(err).toBe(CONFIG_ERROR_INTERACTIVE_MODE);
- expectNoTasksToHaveBeenRun();
+ assertGitError(err, CONFIG_ERROR_INTERACTIVE_MODE, TaskConfigurationError);
+ assertNoExecutedTasks();
 
  done();
  });
  })
  );
  });
 
- function expectNoTasksToHaveBeenRun() {
- assertNoExecutedTasks();
- }
-
- function expectTheError<E extends Error>(err: E | null) {
- return {
- toBe(message: string) {
- expect(err).toBeInstanceOf(TaskConfigurationError);
- expect(String(err)).toMatch(message);
- },
- };
- }
-
  function test(t: (done: Function) => void) {
  return async () => {
  await new Promise((done) => t(done));

simple-git/test/unit/plugin.unsafe.spec.ts

@@ -0,0 +1,97 @@
+import { promiseError } from "@kwsites/promise-result";
+import { assertExecutedCommands, assertGitError, closeWithSuccess, newSimpleGit } from "./__fixtures__";
+
+describe("blockUnsafeOperationsPlugin", () => {
+
+ it.each([
+ ["cmd", "--upload-pack=touch /tmp/pwn0"],
+ ["cmd", "--receive-pack=touch /tmp/pwn1"],
+ ["clone", "-u touch /tmp/pwn"]
+ ])("allows %s %s only when using override", async (cmd, option) => {
+ assertGitError(
+ await promiseError(newSimpleGit({ unsafe: {} }).raw(cmd, option)),
+ "allowUnsafePack"
+ );
+
+ const err = promiseError(
+ newSimpleGit({ unsafe: { allowUnsafePack: true } }).raw(cmd, option)
+ );
+
+ await closeWithSuccess();
+ expect(await err).toBeUndefined();
+ assertExecutedCommands(cmd, option);
+ });
+
+ it("allows -u for non-clone commands", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = promiseError(
+ git.raw("push", "-u", "origin/main")
+ );
+
+ await closeWithSuccess();
+ expect(await err).toBeUndefined();
+ assertExecutedCommands("push", "-u", "origin/main");
+ });
+
+ it("blocks -u for clone command", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = promiseError(
+ git.clone("-u touch /tmp/pwn", "file:///tmp/zero12")
+ );
+
+ assertGitError(await err, "allowUnsafePack");
+ });
+
+ it("allows -u for clone command with override", async () => {
+ const git = newSimpleGit({ unsafe: { allowUnsafePack: true } });
+ const err = promiseError(
+ git.clone("-u touch /tmp/pwn", "file:///tmp/zero12")
+ );
+
+ await closeWithSuccess();
+ expect(await err).toBeUndefined();
+ assertExecutedCommands("clone", "-u touch /tmp/pwn", "file:///tmp/zero12");
+ });
+
+ it("blocks pull --upload-pack", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = await promiseError(
+ git.pull("--upload-pack=touch /tmp/pwn0", "master")
+ );
+
+ assertGitError(err, "allowUnsafePack");
+ });
+
+ it("blocks push --receive-pack", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = await promiseError(
+ git.push("--receive-pack=touch /tmp/pwn1", "master")
+ );
+
+ assertGitError(err, "allowUnsafePack");
+ });
+
+ it("blocks raw --upload-pack", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = await promiseError(git.raw("pull", `--upload-pack=touch /tmp/pwn0`));
+
+ assertGitError(err, "allowUnsafePack");
+ });
+
+ it("blocks raw --receive-pack", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = await promiseError(git.raw("push", `--receive-pack=touch /tmp/pwn1`));
+
+ assertGitError(err, "allowUnsafePack");
+ });
+
+ it("blocks listRemote --upload-pack", async () => {
+ const git = newSimpleGit({ unsafe: {} });
+ const err = await promiseError(
+ git.listRemote(["--upload-pack=touch /tmp/pwn2", "main"])
+ );
+
+ assertGitError(err, "allowUnsafePack");
+ });
+
+});