[PATCH] Bug: race condition in reencrypt_path

[t.fitschen at indiscale.com]

Sorry, Louis, what is tmpdir? It's not a bash built-in, is it? Did I
miss something?

I would suggest using `mktemp` which is as secure as gets when you are
writing unencrypted content on the disk, I suppose. And yes, I think it
is not ideal to do that.

Apart from that, I don't know if fixing a gpg bug here is the way to go.

While I didn't run into this bug yet, I think it is good that you
found a work-around anyways, because I know that there have been some
issues with the locking (https://dev.gnupg.org/T5884), so thank you.

Regards
Timm

On Mon, Dec 05, 2022 at 11:16:24PM +0100, Louis Bettens wrote:
> ---
>  src/password-store.sh | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/password-store.sh b/src/password-store.sh
> index 22e818f..549848e 100755
> --- a/src/password-store.sh
> +++ b/src/password-store.sh
> @@ -110,6 +110,7 @@ set_gpg_recipients() {
>  reencrypt_path() {
>  	local prev_gpg_recipients="" gpg_keys="" current_keys="" index passfile
>  	local groups="$($GPG $PASSWORD_STORE_GPG_OPTS --list-config --with-colons | grep "^cfg:group:.*")"
> +	[[ -d "$SECURE_TMPDIR" ]] || die "Error: secure temporary directory not found"
>  	while read -r -d "" passfile; do
>  		[[ -L $passfile ]] && continue
>  		local passfile_dir="${passfile%/*}"
> @@ -117,7 +118,7 @@ reencrypt_path() {
>  		passfile_dir="${passfile_dir#/}"
>  		local passfile_display="${passfile#$PREFIX/}"
>  		passfile_display="${passfile_display%.gpg}"
> -		local passfile_temp="${passfile}.tmp.${RANDOM}.${RANDOM}.${RANDOM}.${RANDOM}.--"
> +		local passfile_temp="${SECURE_TMPDIR}/passfile.tmp.${RANDOM}.${RANDOM}.${RANDOM}.${RANDOM}.--"
>  
>  		set_gpg_recipients "$passfile_dir"
>  		if [[ $prev_gpg_recipients != "${GPG_RECIPIENTS[*]}" ]]; then
> @@ -133,8 +134,9 @@ reencrypt_path() {
>  
>  		if [[ $gpg_keys != "$current_keys" ]]; then
>  			echo "$passfile_display: reencrypting to ${gpg_keys//$'\n'/ }"
> -			$GPG -d "${GPG_OPTS[@]}" "$passfile" | $GPG -e "${GPG_RECIPIENT_ARGS[@]}" -o "$passfile_temp" "${GPG_OPTS[@]}" &&
> -			mv "$passfile_temp" "$passfile" || rm -f "$passfile_temp"
> +			$GPG -d "${GPG_OPTS[@]}" -o "$passfile_temp" "${GPG_OPTS[@]}" "$passfile" &&
> +			$GPG -e "${GPG_RECIPIENT_ARGS[@]}" -o "$passfile" "${GPG_OPTS[@]}" "$passfile_temp" ||
> +			shred "$passfile_temp"
>  		fi
>  		prev_gpg_recipients="${GPG_RECIPIENTS[*]}"
>  	done < <(find "$1" -path '*/.git' -prune -o -path '*/.extensions' -prune -o -iname '*.gpg' -print0)
> @@ -335,6 +337,8 @@ cmd_init() {
>  	local gpg_id="$PREFIX/$id_path/.gpg-id"
>  	set_git "$gpg_id"
>  
> +	tmpdir #Defines $SECURE_TMPDIR, required for reencrypt_path
> +
>  	if [[ $# -eq 1 && -z $1 ]]; then
>  		[[ ! -f "$gpg_id" ]] && die "Error: $gpg_id does not exist and so cannot be removed."
>  		rm -v -f "$gpg_id" || exit 1
> @@ -624,6 +628,8 @@ cmd_copy_move() {
>  	local interactive="-i"
>  	[[ ! -t 0 || $force -eq 1 ]] && interactive="-f"
>  
> +	tmpdir #Defines $SECURE_TMPDIR, required for reencrypt_path
> +
>  	set_git "$new_path"
>  	if [[ $move -eq 1 ]]; then
>  		mv $interactive -v "$old_path" "$new_path" || exit 1
> -- 
> 2.38.1
> 

-- 
Herr/Mr Timm Fitschen
(er/he)
Development

T: +49 551 288 76 48-3
E: t.fitschen at indiscale.com
I: indiscale.com

IndiScale - Wir machen individuelles Datenmanagement skalierbar.

IndiScale GmbH
Lotzestraße 22a
37083 Göttingen

Amtsgericht Göttingen • HRB 205721
Geschäftsführung Henrik tom Wörden
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 906 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/password-store/attachments/20221206/56741143/attachment.sig>