On Sun, Apr 21, 2024 at 11:03:39AM +0100, Richard W.M. Jones wrote:
> The plugin/filter short name detection is very liberal, disallowing
> only '.' and '/'. Thus, at least in theory, short plugin names
> containing almost arbitrary symbols and characters are permitted.
>
> We should probably reserve more characters, but in this commit I only
> reserve:
>
> * backslash (ie. dir separator on Windows)
> * ':' and ';' (common path separators)
> * space
> * non-printable ASCII characters
>
> Also DIR_SEPARATOR_STR, but that's likely to be already covered by the
> other tests so probably does nothing here.
>
> This commit is mainly about tightening up corner cases with possible
> security implications, for example if you managed to trick a program
> to invoke 'nbdkit "plugin param"' that might have an ambiguous parsing
> that you could use to your advantage. It should have no effect on
> normal, non-adversarial usage.
> ---
> server/options.h | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/server/options.h b/server/options.h
> index 7d0730bae7..55e6abc28c 100644
> --- a/server/options.h
> +++ b/server/options.h
> @@ -117,7 +117,21 @@ static const struct option long_options[] = {
> static inline bool
> is_short_name (const char *filename)
> {
> - return strchr (filename, '.') == NULL && strchr (filename, '/') == NULL;
> + const size_t n = strlen (filename);
> + size_t i;
> +
> + for (i = 0; i < n; ++i) {
> + switch (filename[i]) {
> + case '\0'...31: case 127: /* non-printable ASCII */
Compressed case label ranges are a gcc extension. I guess clang has
it too, and we already require one of those two compilers for
__attribute__((cleanup)), so it's not necessarily fatal. But as it
appears to be our first use of that extension, it may be more
conservative to instead
> + case ' ':
> + case '.':
> + case '/': case '\\': /* directory separators */
> + case ':': case ';': /* path separators */
> + return false;
add 'default: if ((unsigned)filename[i] < 31) return false;'.
> + }
> + }
> +
> + return strstr (filename, DIR_SEPARATOR_STR) == NULL;
> }
>
> #endif /* NBDKIT_OPTIONS_H */
> --
> 2.44.0
>
Reviewed-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list -- guestfs@lists.libguestfs.org
To unsubscribe send an email to guestfs-le...@lists.libguestfs.org
