[PATCH] ash: use memmove instead of mempcpy in subevalvar

[Hadrien Lacour]

On Sat, Feb 26, 2022 at 06:11:30PM +0000, Ariadne Conill wrote:
> While investigating a sporadic crash issue relating to variable substitution in
> Alpine Linux, we managed to get a reliable crash when building BusyBox with ASan,
> due to the source and destination overlapping for mempcpy, which resulted in
> sporadic data corruption outside ASan.
>
> Per POSIX, memcpy is not allowed to overlap source and destination, as mempcpy
> is a GNU-specific extension to mempcpy, the same semantics can be assumed.
> Accordingly, we use memmove instead, which does not have this limitation.
>
> Signed-off-by: Ariadne Conill <ariadne at dereferenced.org>
> ---
>  shell/ash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index adb0f223a..6f256d4c3 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7187,7 +7187,7 @@ subevalvar(char *start, char *str, int strloc,
>  			len = orig_len - pos;
>
>  		if (!quotes) {
> -			loc = mempcpy(startp, startp + pos, len);
> +			loc = memmove(startp, startp + pos, len);
>  		} else {
>  			for (vstr = startp; pos != 0; pos--) {
>  				if ((unsigned char)*vstr == CTLESC)
> --
> 2.35.1
>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

Looks like the returned pointer isn't the same between the two functions;
shouldn't it be `loc = memmove(...) + len` instead ?