Demi Marie Obenour <demiobenour@gmail.com> writes:
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.
I see. I suppose it comes down to whether not running dirname speeds things up enough to justify it.
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?
I'm not sure! I've not heard of such a thing.
+ (-*) + 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.
The only place I see where we hardcode a store path is host/initramfs/default.nix, which is a bug and easy to fix with Nix code. Of course you wouldn't reproduce the same image if you built with a different store directory, but it shouldn't be invalid to do so.