echo -e and -n cannot portably be used

[calestyo]

Hey.

POSIX’ echo implements no parameters, neither -e nor -n. As noticed in https://github.com/digint/btrbk/issues/503#issuecomment-1316079503 my Debian system actually prints out:

-e -n #btrbk-v0.32.5

in the sidecar files.

echo should be replaced by printf (which is also built-in in most shells). I could in principle try to provide a PR, but there are a number of things I don't know:

1. Is -e really needed in those places: https://github.com/digint/btrbk/blob/125b37468a518f282e0fafa5dcc7713cf37ed736/contrib/cron/btrbk-mail#L97-L100 https://github.com/digint/btrbk/blob/125b37468a518f282e0fafa5dcc7713cf37ed736/contrib/cron/btrbk-verify#L216 https://github.com/digint/btrbk/blob/125b37468a518f282e0fafa5dcc7713cf37ed736/btrbk#L2095 IMO it always feels a bit dangerous if one uses -e as depending on the input and further usage, and attacker could try to generate "evil" characters like ; or so via \xXX escapes.

2. Can’t we simply use find’s -print here? https://github.com/digint/btrbk/blob/125b37468a518f282e0fafa5dcc7713cf37ed736/btrbk#L2014

Cheers, Chris.

[digint]

fixed echo (late yesterday) for btrbk in f52de197d6a9ea28e0333fcd7187f6eff0359b0d.

btrbk-verify and btrbk-maill are bash scripts, where echo is built-in

[calestyo]

Well.. still no harm in getting rid of needless non-POSIX stuff?

What about the usage in find ... and have you considered my concerns that escaped stuff might be abused? I mean this problem persists, when you use printf like printf "$somestring" and not like printf %s "$somestring"... the quoteshell may not change this.

[digint]

removed all echo -e in: b2e927e19ef73567f5424bc5f9d482373fe0fd0b, 810f8969d0f7f4b6b44b3cdad24bdaef1cecdad1

improved find in: c6b370d8190c6105f5a8ff49a303b077176478dd (will be merged after some more testing)

[calestyo]

Thanks... but:

https://github.com/digint/btrbk/blob/b2e927e19ef73567f5424bc5f9d482373fe0fd0b/contrib/cron/btrbk-mail#L59 you could have gotten that much easier by using printf. There you could directly use \n in the strings and print via printf '%s\n' "$string" (which does not interpret any escapes in $string which is the exact equivalent to echo "$string".

And printf is also built-in to all sane shells.

This one I either don’t understand or seems pretty grave: https://github.com/digint/btrbk/blob/b2e927e19ef73567f5424bc5f9d482373fe0fd0b/contrib/cron/btrbk-mail#L99

Why do you arbitrarily overwrite a file in /tmp? Debug leftover? 😅

[digint]

Why do you arbitrarily overwrite a file in /tmp? Debug leftover? sweat_smile

debug leftover... force-pushed changes, thanks

[calestyo]

Thanks.

Still unsure whether I understand this one correctly: https://github.com/digint/btrbk/blob/f52de197d6a9ea28e0333fcd7187f6eff0359b0d/btrbk#L2095

You know that when you directly give the string as first argument to printf (i.e. not as in printf '%s' "$str") it will interpret any %- and \-escapes in it?

Is this still guaranteed to be safe, with the possible values of @line? E.g when a distributor overrides $VERSION or should the filenames (ever) be allowed to contain \ or %?

[digint]

no, was not guaranteed to be safe (missed to take this into account when allowing special chars in files). Fixed in: f9c7a47b6a89e8bfd0818692db0577da01d1de3b

[calestyo]

Hey… and sorry for having to ask again.

I guess it's just because I don't understand perl respectively your code deeply enough,... but even with f9c7a47, you don’t seem to use a fixed format string for printf like %s\n or so, but give some "arbitrary" strings to it as the format string.

quoteshell seems to only put single quotes ' around any string and escape any literal ', so e.g. the string fo'o is escaped as 'fo'\''o'.

But this still allows for arbitrary \ and % to be included, without being escaped by the function, and thus ending up in the format string for the printf, e.g.:

quoteshell("fo\\no%dx")

would result in 'fo\no%dx' as the format string for printf, where \n is interpreted as newline an %d would expect an integer parameter, e.g.:

$ printf 'fo\no%dx'

gives:

fo
o0x

with a newline, and with the 0 coming from the non-set parameter (argument #2 in the command) for the %d.

E.g.:

$ printf 'fo\no%dx' 21

gives:

fo
o21x

[digint]

Yes, you are right, but:

https://github.com/digint/btrbk/blob/f9c7a47b6a89e8bfd0818692db0577da01d1de3b/btrbk#L2097-L2100

in @line, I assemble the FORMAT, containing only the keys from keys %raw_info (hash keys, i.e. "FILE", "compress", ...), which I control, contrained to: /[a-zA-Z-]+/.

https://github.com/digint/btrbk/blob/f9c7a47b6a89e8bfd0818692db0577da01d1de3b/btrbk#L338-L350 https://github.com/digint/btrbk/blob/f9c7a47b6a89e8bfd0818692db0577da01d1de3b/btrbk#L1763

The values from @subst (especially FILE, wich I don't control), are passed as ARGUMENTS to printf.

# printf 'FILE=%s' 'evil \n escapes \\n and '\''ticks'
FILE=evil \n escapes \\n and 'ticks

output from btrbk:

# ./btrbk  -c /tmp/btrbk_test/btrbk.conf run -v -v 2>&1 | grep \#\#\#
[...]
### printf '#btrbk-v0.32.6-dev\n# Do not edit this file\n#t=1668985609\nTYPE=%s\nFILE=%s\nRECEIVED_UUID=%s\nRECEIVED_PARENT_UUID=%s\ncompress=%s\nINCOMPLETE=%s\n' 'raw' 'svol.20221121.btrfs.xz' '5bb7e895-ff53-fe4e-aee1-8c5b68e257f7' 'b1478e76-d931-6a40-8a62-aeffedd448e9' 'xz' '1' > '/tmp/btrbk_test/mnt_target/svol.20221121.btrfs.xz.info'
[...]

[digint]

@calestyo a bit off-topic, today I hacked around in action-cp branch: try:

btrbk cp svol.20221120.btrfs.xz /path/to/btrfs/fs/

should magically restore your subvolume ;-)

[calestyo]

should magically restore your subvolume ;-)

You mean from a remote backup... to a local subvol? That would sound nice 😍. Would that work with incremental dumps... and also when there's encryption?