On Mon, Apr 22, 2024 at 12:58:10PM -0500, Eric Blake wrote:
On Sun, Apr 21, 2024 at 08:15:42PM +0100, Richard W.M. Jones wrote:
> The plugin/filter short name detection is very liberal, reserving only
> '.' and '/'. Thus, at least in theory, short plugin names
containing
> almost arbitrary symbols and characters are permitted.
>
> Backslash ought to have been reserved when we added Windows support.
>
> We should probably reserve more characters, but in this commit I only
> reserve:
>
> * backslash (ie. directory separator on Windows)
> * ':' and ';' (common path separators)
> * '=' (used in nbdkit parameters)
Oh, I reviewed v1 before you added '=' to the reject list in v2.
> * space and comma (commonly used to separate lists)
> * 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 | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> +
> + for (i = 0; i < n; ++i) {
> + switch (filename[i]) {
> + case '\0'...31: case 127: /* non-printable ASCII */
The comment about ranged case label is still present (I'm okay whether
you keep it in or explode it to long-hand).
I'll change it to:
+ /* non-printable ASCII */
+ case 127:
+ return false;
+ default:
+ if ((unsigned)filename[i] <= 31)
+ return false;
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW