[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200329025436.GS11469@brightrain.aerifal.cx>
Date: Sat, 28 Mar 2020 22:54:36 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 1/4] ldso: add option to rewrite the argv block
On Sat, Mar 28, 2020 at 07:19:25PM -0500, rcombs wrote:
> ---
> ldso/dynlink.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
> index 6468f20..c378f00 100644
> --- a/ldso/dynlink.c
> +++ b/ldso/dynlink.c
> @@ -1698,6 +1698,7 @@ void __dls3(size_t *sp, size_t *auxv)
> char **argv = (void *)(sp+1);
> char **argv_orig = argv;
> char **envp = argv+argc+1;
> + int replace_argv = 0;
>
> /* Find aux vector just past environ[] and use it to initialize
> * global data that may be needed before we can make syscalls. */
> @@ -1771,6 +1772,8 @@ void __dls3(size_t *sp, size_t *auxv)
> if (opt[5]=='=') replace_argv0 = opt+6;
> else if (opt[5]) *argv = 0;
> else if (*argv) replace_argv0 = *argv++;
> + } else if (!memcmp(opt, "replace-argv", 12)) {
> + replace_argv = 1;
> } else {
> argv[0] = 0;
> }
> @@ -1949,6 +1952,22 @@ void __dls3(size_t *sp, size_t *auxv)
> debug.state = 0;
> _dl_debug_state();
>
> + if (replace_argv) {
> + char *argv_end = argv_orig[0];
> + char *orig_ptr = argv_orig[0];
> + int i;
> + for (i = 0; i < (int)(argc - (argv-argv_orig)); i++) {
> + char *src = (i == 0 && replace_argv0) ? replace_argv0 : argv[i];
> + int len = strlen(src) + 1;
> + memmove(orig_ptr, src, len);
> + argv_end = argv[i] + strlen(argv[i]);
> + argv[i] = orig_ptr;
> + orig_ptr += len;
> + }
> + for (; orig_ptr < argv_end; orig_ptr++)
> + *orig_ptr = 0;
> + }
> +
> if (replace_argv0) argv[0] = replace_argv0;
>
> errno = 0;
> --
> 2.7.4
Can you clarify what the purpose of this patch/option is? It seems
unrelated to the rest of the series and looks like it's doing
something really sketchy. It looks like it's making assumption about
the layout of the original strings, which is not an interface
contract, and like it happily overflows and clobbers unrelated memory
if replace_argv0 is longer than the original string pointed to by
argv[0].
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
