linux-kernel - Re: [PATCH] param: fixup quote parsing of kernel arguments

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87twwr17ow.fsf@rustcorp.com.au>
Date:	Wed, 08 Apr 2015 15:29:43 +0930
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Arthur Gautier <baloo@...di.net>
Cc:	linux-kernel@...r.kernel.org, Arthur Gautier <baloo@...di.net>
Subject: Re: [PATCH] param: fixup quote parsing of kernel arguments

Arthur Gautier <baloo@...di.net> writes:
> When starting kernel with arguments like:
>   init=/bin/sh -c "echo arguments"
> the trailing double quote is not removed which results in following command
> being executed:
>   /bin/sh -c 'echo arguments"'
>
> This commit removes the trailing double quote.
>
> Signed-off-by: Arthur Gautier <baloo@...di.net>

Hi Arthur,

        Thanks, I'd not considered quotes outside '='.  But this
fixes it in a weird way: we handle quotes below, we just don't do
anything for the "raw value" case:

	for (i = 0; args[i]; i++) {
		if (isspace(args[i]) && !in_quote)
			break;
		if (equals == 0) {
			if (args[i] == '=')
				equals = i;
		}
		if (args[i] == '"')
			in_quote = !in_quote;
	}

	*param = args;
	if (!equals)
		*val = NULL;
	else {
		args[equals] = '\0';
		*val = args + equals + 1;

		/* Don't include quotes in value. */
		if (**val == '"') {
			(*val)++;
			if (args[i-1] == '"')
				args[i-1] = '\0';
		}
		if (quoted && args[i-1] == '"')
			args[i-1] = '\0';
	}

The logical fix is to just always remove the close quotes in both
cases:

diff --git a/kernel/params.c b/kernel/params.c
index 728e05b167de..a22d6a759b1a 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -173,9 +173,9 @@ static char *next_arg(char *args, char **param, char **val)
 			if (args[i-1] == '"')
 				args[i-1] = '\0';
 		}
-		if (quoted && args[i-1] == '"')
-			args[i-1] = '\0';
 	}
+	if (quoted && args[i-1] == '"')
+		args[i-1] = '\0';
 
 	if (args[i]) {
 		args[i] = '\0';

Does this work for you?

Thanks,
Rusty.

> ---
>  kernel/params.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index 728e05b..2118546 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -156,8 +156,11 @@ static char *next_arg(char *args, char **param, char **val)
>  			if (args[i] == '=')
>  				equals = i;
>  		}
> -		if (args[i] == '"')
> +		if (args[i] == '"') {
> +			if (!equals)
> +				args[i] = '\0';
>  			in_quote = !in_quote;
> +		}
>  	}
>  
>  	*param = args;
> -- 
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/