On 9/8/25 04:36, Alyssa Ross wrote:
Demi Marie Obenour <demiobenour@gmail.com> writes:
This isn't a security feature as the input is trusted, but it might catch some bugs in the future. Additionally, it will allow replacing an external command with builtin string manipulation, as paths that the builtin manipulation would mishandle will instead be rejected.
In general this feels a bit overkill to me, but it depends — have you encountered bugs this would help prevent?
No, but it does make me more confident about omitting calls to an external dirname command, which should speed stuff up.
Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com> --- scripts/make-erofs.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/scripts/make-erofs.sh b/scripts/make-erofs.sh index e63bcbed9c3028f0f2b55431d46ba9ec67bc26ef..cf942972910c76e1835dc5b0084c2d04bf084a9d 100755 --- a/scripts/make-erofs.sh +++ b/scripts/make-erofs.sh @@ -28,6 +28,34 @@ trap 'chmod -R +w -- "$root" && rm -rf -- "$superroot"' EXIT root=$superroot/real_root mkdir -- "$root"
+check_path () { + # Various code can only handle paths that do not end with / + # and are in canonical form. Reject others. + for i; do + case $i in + (''|.|..|./*|../*|*/|*/.|*/..|*//*|*/./*|*/../*) + printf 'Path "%s" is /, //, empty, or not canonical\n' "$i" >&2 + exit 1 + ;; + (*[!A-Za-z0-9._@+/-]*) + printf 'Path "%s" has forbidden characters\n' "$i" >&2 + exit 1 + ;;
Not sure why we'd want to rule out most characters? We're not really in control of what characters packages choose to use in their store paths.
I believe Nix has an allowlist of permitted characters in store paths. Is this documented, or is it just in the C++ source code?
+ (-*) + printf 'Path "%s" begins with -\n' "$i" >&2 + exit 1 + ;; + (/nix/store/*|[!/]*)
It's technically possible to use Nix with a different store path, so I'd like to avoid anything that requires us to hardcode /nix/store.
Right now, the generated images depend on the store paths, so the scripts would need to be adapted to support this. If we are going to generalize this, I recommend using a proper scripting language like Python, Perl, or Lua.
+ : + ;; + (*) + printf 'Path "%s" is neither relative nor a Nix store path\n' "$i" >&2 + exit 1 + ;; + esac + done +} + while read -r arg1; do read -r arg2 || ex_usage
@@ -38,6 +66,7 @@ while read -r arg1; do echo
if [ "$arg2" = / ]; then + check_path "$arg1" cp -RT -- "$arg1" "$root" # Nix store paths are read-only, so fix up permissions # so that subsequent copies can write to directories @@ -47,6 +76,8 @@ while read -r arg1; do continue fi
+ check_path "$arg1" "$arg2" + parent=$(dirname "$arg2") mkdir -p -- "$root/$parent" cp -RT -- "$arg1" "$root/$arg2"
-- 2.51.0
-- Sincerely, Demi Marie Obenour (she/her/hers)