Merge pull request from GHSA-jvmr-5fpx-r76j

* Enforce allowlist for query parameters

Also mitigate other argument injection

* Add allow-remote-command option

src/main.ts

@@ -102,7 +102,12 @@ const opts = yargs(hideBin(process.argv))
   })
   .option('allow-remote-hosts', {
     description:
-      'Allow WeTTY to use the `host` param in a url as ssh destination',
+      'Allow WeTTY to use the `host` and `port` params in a url as ssh destination',
+    type: 'boolean',
+  })
+  .option('allow-remote-command', {
+    description:
+      'Allow WeTTY to use the `command` and `path` params in a url as command and working directory on ssh host',
     type: 'boolean',
   })
   .option('log-level', {

src/server.ts

@@ -6,9 +6,7 @@ import express from 'express';
 import gc from 'gc-stats';
 import { Gauge, collectDefaultMetrics } from 'prom-client';
 import { getCommand } from './server/command.js';
-import { login } from './server/login.js';
 import { gcMetrics } from './server/metrics.js';
-import { escapeShell } from './server/shared/shell.js';
 import { server } from './server/socketServer.js';
 import { spawn } from './server/spawn.js';
 import {
@@ -75,17 +73,11 @@ export async function decorateServerWithSsh(
      * @name connection
      */
     logger.info('Connection accepted.');
-    const [args, sshUser] = getCommand(socket, ssh, command, forcessh);
-    const cmd = args.join(' ');
-    logger.debug('Command Generated', { user: sshUser, cmd });
     wettyConnections.inc();
 
     try {
-      if (!sshUser) {
-        const username = await login(socket);
-        args[1] = `${escapeShell(username.trim())}@${args[1]}`;
-        logger.debug('Spawning term', { username: username.trim(), cmd });
-      }
+      const args = await getCommand(socket, ssh, command, forcessh);
+      logger.debug('Command Generated', { cmd: args.join(' ') });
       await spawn(socket, args);
     } catch (error) {
       logger.info('Disconnect signal sent', { err: error });

src/server/command.ts

@@ -14,17 +14,23 @@ const localhost = (host: string): boolean =>
 
 const urlArgs = (
   referer: string | undefined,
-  def: { [s: string]: string },
+  {
+    allowRemoteCommand,
+    allowRemoteHosts,
+  }: {
+    allowRemoteCommand: boolean;
+    allowRemoteHosts: boolean;
+  },
 ): { [s: string]: string } =>
-  Object.assign(def, url.parse(referer || '', true).query);
+  _.pick(
+    _.pickBy(url.parse(referer || '', true).query, _.isString),
+    ['pass'],
+    allowRemoteCommand ? ['command', 'path'] : [],
+    allowRemoteHosts ? ['port', 'host'] : [],
+  );
 
-export function getCommand(
-  {
-    request: { headers },
-    client: {
-      conn: { remoteAddress },
-    },
-  }: Socket,
+export async function getCommand(
+  socket: Socket,
   {
     user,
     host,
@@ -35,29 +41,30 @@ export function getCommand(
     knownHosts,
     config,
     allowRemoteHosts,
+    allowRemoteCommand,
   }: SSH,
   command: string,
-  forcessh: boolean,
-): [string[], boolean] {
-  const sshAddress = address(headers, user, host);
+  forcessh: boolean
+): Promise<string[]> {
+  const {
+    request: { headers: { referer } },
+    client: { conn: { remoteAddress } },
+  } = socket;
+
   if (!forcessh && localhost(host)) {
-    return [loginOptions(command, remoteAddress), true];
+    return loginOptions(command, remoteAddress);
   }
-  const args = urlArgs(headers.referer, {
+
+  const sshAddress = await address(socket, user, host);
+  const args = {
     host: sshAddress,
     port: `${port}`,
     pass: pass || '',
     command,
     auth,
     knownHosts,
     config: config || '',
-  });
-  if (!allowRemoteHosts) {
-    args.host = sshAddress;
-  }
-
-  return [
-    sshOptions(args, key),
-    user !== '' || user.includes('@') || sshAddress.includes('@'),
-  ];
+    ...urlArgs(referer, { allowRemoteHosts, allowRemoteCommand }),
+  };
+  return sshOptions(args, key);
 }

src/server/command/address.ts

@@ -1,23 +1,32 @@
 import _ from 'lodash';
+import { Socket } from 'socket.io';
+import { login } from '../login.js';
 import { escapeShell } from '../shared/shell.js';
-import type { IncomingHttpHeaders } from 'http';
 
-export function address(
-  headers: IncomingHttpHeaders,
+export async function address(
+  socket: Socket,
   user: string,
   host: string,
-): string {
+): Promise<string> {
   // Check request-header for username
-  const remoteUser = headers['remote-user'];
-  if (!_.isUndefined(remoteUser) && !Array.isArray(remoteUser)) {
-    return `${escapeShell(remoteUser)}@${host}`;
-  }
-  if (!_.isUndefined(headers.referer)) {
-    const match = headers.referer.match('.+/ssh/([^/]+)$');
-    if (match) {
-      const username = escapeShell(match[1].split('?')[0]);
-      return `${username}@${host}`;
+  const { request: { headers: {
+    'remote-user': userFromHeader,
+    referer
+  } } } = socket;
+
+  let username: string | undefined;
+  if (!_.isUndefined(userFromHeader) && !Array.isArray(userFromHeader)) {
+    username = userFromHeader;
+  } else {
+    const userFromPathMatch = referer?.match('.+/ssh/([^/]+)$');
+    if (userFromPathMatch) {
+      // eslint-disable-next-line prefer-destructuring
+      username = userFromPathMatch[1].split('?')[0];
+    } else if (user) {
+      username = user;
+    } else {
+      username = await login(socket);
     }
   }
-  return user ? `${escapeShell(user)}@${host}` : host;
+  return `${escapeShell(username)}@${host}`;
 }

src/server/command/ssh.ts

@@ -17,32 +17,22 @@ export function sshOptions(
   const cmd = parseCommand(command, path);
   const hostChecking = knownHosts !== '/dev/null' ? 'yes' : 'no';
   logger().info(`Authentication Type: ${auth}`);
-  let sshRemoteOptsBase = ['ssh', host, '-t'];
-  if (!isUndefined(config) && config !== '') {
-    sshRemoteOptsBase = sshRemoteOptsBase.concat(['-F', config]);
-  }
-  if (!isUndefined(port) && port !== '') {
-      sshRemoteOptsBase = sshRemoteOptsBase.concat(['-p', port]);
-  }
-  sshRemoteOptsBase = sshRemoteOptsBase.concat([
-    '-o',
-    `PreferredAuthentications=${auth}`,
-    '-o',
-    `UserKnownHostsFile=${knownHosts}`,
-    '-o',
-    `StrictHostKeyChecking=${hostChecking}`,
-  ]);
-  if (!isUndefined(key)) {
-    return sshRemoteOptsBase.concat(['-i', key, cmd]);
-  }
-  if (pass !== '') {
-    return ['sshpass', '-p', pass].concat(sshRemoteOptsBase, [cmd]);
-  }
-  if (auth === 'none') {
-    sshRemoteOptsBase.splice(sshRemoteOptsBase.indexOf('-o'), 2);
-  }
 
-  return cmd === '' ? sshRemoteOptsBase : sshRemoteOptsBase.concat([cmd]);
+  return [
+    ...pass ? ['sshpass', '-p', pass] : [],
+    'ssh',
+    '-t',
+    ...config ? ['-F', config] : [],
+    ...port ? ['-p', port] : [],
+    ...key ? ['-i', key] : [],
+    ...auth !== 'none' ? ['-o', `PreferredAuthentications=${auth}`] : [],
+    '-o', `UserKnownHostsFile=${knownHosts}`,
+    '-o', `StrictHostKeyChecking=${hostChecking}`,
+    '-o', 'EscapeChar=none',
+    '--',
+    host,
+    ...cmd ? [cmd] : [],
+  ];
 }
 
 function parseCommand(command: string, path?: string): string {

src/server/shared/shell.spec.ts

@@ -17,4 +17,9 @@ describe('Values passed to escapeShell should be safe to pass woth sub processes
     const cmd = escapeShell("-oProxyCommand='bash' -c `wget localhost:2222`");
     expect(cmd).to.equal('oProxyCommandbash-cwgetlocalhost2222');
   });
+
+  it('should remove dashes even when there are illegal characters before them', () => {
+    const cmd = escapeShell("`-oProxyCommand='bash' -c `wget localhost:2222`");
+    expect(cmd).to.equal('oProxyCommandbash-cwgetlocalhost2222');
+  });
 });

src/server/shared/shell.ts

@@ -1,3 +1,3 @@
 export const escapeShell = (username: string): string =>
   // eslint-disable-next-line no-useless-escape
-  username.replace(/^-|[^a-zA-Z0-9_\\\-\.\@-]/g, '');
+  username.replace(/[^a-zA-Z0-9_\\\-\.\@-]/g, '').replace(/^-+/g, '');

src/shared/config.ts

@@ -138,6 +138,7 @@ export function mergeCliConf(opts: Arguments, config: Config): Config {
       pass: opts['ssh-pass'],
       key: opts['ssh-key'],
       allowRemoteHosts: opts['allow-remote-hosts'],
+      allowRemoteCommand: opts['allow-remote-command'],
       config: opts['ssh-config'],
       knownHosts: opts['known-hosts'],
     }) as SSH,

src/shared/defaults.ts

@@ -10,6 +10,7 @@ export const sshDefault: SSH = {
   port: parseInt(process.env.SSHPORT || '22', 10),
   knownHosts: process.env.KNOWNHOSTS || '/dev/null',
   allowRemoteHosts: false,
+  allowRemoteCommand: false,
   config: process.env.SSHCONFIG || undefined,
 };
 

src/shared/interfaces.ts

@@ -8,6 +8,7 @@ export interface SSH {
   port: number;
   knownHosts: string;
   allowRemoteHosts: boolean;
+  allowRemoteCommand: boolean;
   pass?: string;
   key?: string;
   config?: string;