From ddc9b810dea6f34f3f24a30127f3ab02c7fbbf4f Mon Sep 17 00:00:00 2001 From: Christoph Anton Mitterer Date: Mon, 21 Nov 2022 04:33:10 +0100 Subject: [PATCH] ssh_filter_btrbk.sh: convert to POSIX sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit finishes the work from the previous one and converts ssh_filter_btrbk.sh to (mostly) pure POSIX Shell Command Language. Instead of bash’s `=~`-operator for its `[[ … ]]`-compound-command it uses `grep`. At the time of writing, bash has at least the `nocasematch`-shell-option which would have a negatve security impact for this program. While it’s not enabled per default single users could potentially change that, not realising the consequences. Thus, moving away from this may also provide some hardening. Unlike bash’s `=~`-operator, which matches against the whole string at once, `grep` matches the pattern against each line of input. This would allow for attacks by including a newline in the SSH command like in: SSH_ORIGINAL_COMMAND="readlink /dev/stdout cat /etc/shadow" but is prevented by the general exclusion of newlines in commit TODO. `grep` may return an exit status of `0` when used with its `-q`-option, even when an error occurred. Since this program is intended specifically for security purposes this shall be avoided, even if such case is unlikely, and therefore its standard output and standard error are redirected to `/dev/null` instead. Further, using just: local formatted_restrict_path_list="$(printf '%s' "$restrict_path_list" | sed 's/|/", "/g')" rather than: local formatted_restrict_path_list=""; formatted_restrict_path_list="$(printf '%s' "$restrict_path_list" | sed 's/|/", "/g')" prevent `set -e` to take effect if the pipeline within the command substitution fails, as the returned exit status of the whole command is the result of `local`, not that of the assignment. This is however no security problem here, as `formatted_restrict_path_list` is only used for informative pruposes. Signed-off-by: Christoph Anton Mitterer --- ssh_filter_btrbk.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ssh_filter_btrbk.sh b/ssh_filter_btrbk.sh index 97f6848..5cd9c6e 100755 --- a/ssh_filter_btrbk.sh +++ b/ssh_filter_btrbk.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/bin/sh set -e set -u @@ -87,17 +87,21 @@ reject_filtered_cmd() stream_in_match="(${decompress_match} \| )?(${mbuffer_match} \| )?" stream_out_match="( \| ${mbuffer_match})?( \| ${compress_match}$)?" + # `grep`’s `-q`-option is not used as it may cause an exit status of `0` even + # when an error occurred. + allow_stream_match="^${stream_in_match}${allow_cmd_match}${stream_out_match}" - if [[ $SSH_ORIGINAL_COMMAND =~ $allow_stream_match ]] ; then + if printf '%s' "$SSH_ORIGINAL_COMMAND" | grep -E "$allow_stream_match" >/dev/null 2>/dev/null; then return 0 fi exact_cmd_match="^(${allow_exact_list})$"; - if [[ $SSH_ORIGINAL_COMMAND =~ $exact_cmd_match ]] ; then + if printf '%s' "$SSH_ORIGINAL_COMMAND" | grep -E "$exact_cmd_match" >/dev/null 2>/dev/null; then return 0 fi - reject_and_die "disallowed command${restrict_path_list:+ (restrict-path: \"${restrict_path_list//|/\", \"}\")}" + local formatted_restrict_path_list="$(printf '%s' "$restrict_path_list" | sed 's/|/", "/g')" + reject_and_die "disallowed command${restrict_path_list:+ (restrict-path: \"$formatted_restrict_path_list\")}" }