[DTrace-devel] [dtrace-utils REVIEW] dtrace: correct snprintf maximum lengths in, libdtrace/dt_printf.c

Kris Van Hees kris.van.hees at oracle.com
Wed Oct 16 20:29:22 PDT 2019


Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

Please commit this to the dtrace-utils master branch and the most recent
release branch (1.2-branch).  I will apply it to my dtrace v2 dev branch
as well since it affects the new development code as well (the changes
occur in code that thus far is expected to remain in the new version).

Thanks for preparing this patch.

	Kris

On Wed, Oct 16, 2019 at 01:17:28PM -0700, David Mc Lean wrote:
> >From bee39100f9f3d80da7a118bf9a174f1230893949 Mon Sep 17 00:00:00 2001
> From: "David P. Mc Lean" <david.mclean at oracle.com>
> Date: Wed, 4 Sep 2019 11:44:55 -0700
> Subject: [PATCH] dtrace: correct snprintf maximum lengths in
>  libdtrace/dt_printf.c
> 
> dt_printf_format() and dtrace_printf_format() had improperly specified
> values for the maximum size argument of snprintf.
> 
> dt_printf_format() has two instances of snprintf where the destination
> pointer was not at the beginning of the buffer space, while the maximum
> size argument specified the total size of the buffer rather than the
> remaining length.  This has not been a practical problem because the
> output string has always been much shorter than the remaining length of the
> buffer.
> 
> Tests that failed without the correction of this commit are:
>     test/unittest/aggs/tst.keysort.d
>     test/unittest/multiaggs/tst.many.d
>     test/unittest/multiaggs/tst.sort.d
>     test/unittest/multiaggs/tst.sortpos.d
>     test/unittest/multiaggs/tst.tuplecompat.d
>     test/unittest/multiaggs/tst.zero.d
>     test/unittest/multiaggs/tst.zero2.d
>     test/unittest/printa/tst.dynwidth.d
>     test/unittest/printa/tst.stack.d
>     test/unittest/printf/tst.flags.d
>     test/unittest/printf/tst.widths.d
>     test/unittest/printf/tst.widths1.d
>     test/unittest/printf/tst.wp.d
>     test/unittest/rates/tst.aggrate.d
>     test/unittest/rates/tst.switchrate.d
>     test/demo/intro/trussrw.d
>     test/demo/struct/rwinfo.d
> 
> The error seen while running on Ubuntu 19.04 was:
> "*** buffer overflow detected ***: dtrace terminated"
> 
> In each case a core dump file was generated.
> 
> Similarly, the maximum length argument was seen to be incorrect for snprintf
> in the function dtrace_printf_format().  Correct these issues.
> 
> Orabug: 30404549
> Signed-off-by: David  Mc Lean <david.mclean at oracle.com>
> ---
>  libdtrace/dt_printf.c                        | 12 ++++---
>  test/unittest/printf/tst.wide-bug30404549.r  |  4 +++
>  test/unittest/printf/tst.wide-bug30404549.sh | 28 +++++++++++++++
>  test/unittest/printf/tst.wide.r              |  4 +++
>  test/unittest/printf/tst.wide.sh             | 36 ++++++++++++++++++++
>  5 files changed, 80 insertions(+), 4 deletions(-)
>  create mode 100644 test/unittest/printf/tst.wide-bug30404549.r
>  create mode 100755 test/unittest/printf/tst.wide-bug30404549.sh
>  create mode 100644 test/unittest/printf/tst.wide.r
>  create mode 100755 test/unittest/printf/tst.wide.sh
> 
> diff --git a/libdtrace/dt_printf.c b/libdtrace/dt_printf.c
> index e567c6f9..959f8b17 100644
> --- a/libdtrace/dt_printf.c
> +++ b/libdtrace/dt_printf.c
> @@ -1461,10 +1461,12 @@ dt_printf_format(dtrace_hdl_t *dtp, FILE *fp, const dt_pfargv_t *pfv,
>  			width = 0;
>  
>  		if (width != 0)
> -			f += snprintf(f, sizeof (format), "%d", ABS(width));
> +			f += snprintf(f, sizeof (format) - (f - format), "%d",
> +				      ABS(width));
>  
>  		if (prec > 0)
> -			f += snprintf(f, sizeof (format), ".%d", prec);
> +			f += snprintf(f, sizeof (format) - (f - format), ".%d",
> +				      prec);
>  
>  		(void) strcpy(f, pfd->pfd_fmt);
>  		pfd->pfd_rec = rec;
> @@ -1758,10 +1760,12 @@ dtrace_printf_format(dtrace_hdl_t *dtp, void *fmtdata, char *s, size_t len)
>  			*f++ = '@';
>  
>  		if (width != 0)
> -			f += snprintf(f, sizeof (format), "%d", width);
> +			f += snprintf(f, formatlen - (f - format), "%d",
> +				      width);
>  
>  		if (prec != 0)
> -			f += snprintf(f, sizeof (format), ".%d", prec);
> +			f += snprintf(f, formatlen - (f - format), ".%d",
> +				      prec);
>  
>  		/*
>  		 * If the output format is %s, then either %s is the underlying
> diff --git a/test/unittest/printf/tst.wide-bug30404549.r b/test/unittest/printf/tst.wide-bug30404549.r
> new file mode 100644
> index 00000000..7762f847
> --- /dev/null
> +++ b/test/unittest/printf/tst.wide-bug30404549.r
> @@ -0,0 +1,4 @@
> +10000000
> +10000000
> +10000000
> +0
> diff --git a/test/unittest/printf/tst.wide-bug30404549.sh b/test/unittest/printf/tst.wide-bug30404549.sh
> new file mode 100755
> index 00000000..7605a94a
> --- /dev/null
> +++ b/test/unittest/printf/tst.wide-bug30404549.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +
> +#
> +# 30404549 DTrace correct the snprintf maximum lengths in libdtrace/dt_printf.c
> +#
> +
> +if [ $# != 1 ]; then
> +	echo expected one argument: '<'dtrace-path'>'
> +	exit 2
> +fi
> +dtrace=$1
> +
> +# abbreviate output to a simple statement of length.  raw output is big (29M)
> +$dtrace -qs /dev/stdin << EOF | awk '{ print length($0); }'
> +BEGIN
> +{
> +	printf("%10000000d\n", 1);
> +	printf("%.10000000d\n", 1);
> +	printf("%10000000.10000000d\n", 1);
> +	exit(0);
> +}
> +EOF
> diff --git a/test/unittest/printf/tst.wide.r b/test/unittest/printf/tst.wide.r
> new file mode 100644
> index 00000000..5c3cc48f
> --- /dev/null
> +++ b/test/unittest/printf/tst.wide.r
> @@ -0,0 +1,4 @@
> +{9999999* }1
> +{9999999*0}1
> +{9999999*0}1
> +
> diff --git a/test/unittest/printf/tst.wide.sh b/test/unittest/printf/tst.wide.sh
> new file mode 100755
> index 00000000..2c3a3197
> --- /dev/null
> +++ b/test/unittest/printf/tst.wide.sh
> @@ -0,0 +1,36 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +
> +#
> +# 30404549 DTrace correct the snprintf maximum lengths in libdtrace/dt_printf.c
> +#
> +
> +if [ $# != 1 ]; then
> +	echo expected one argument: '<'dtrace-path'>'
> +	exit 2
> +fi
> +dtrace=$1
> +
> +# abbreviate output, where the raw output of prints is big (29M)
> +$dtrace -qs /dev/stdin << EOF | awk '{
> +  if (match($0, "  +"))
> +    printf("%s{%d* }%s\n", substr($0, 1, RSTART - 1),
> +      RLENGTH, substr($0, RSTART + RLENGTH));
> +  else if (match($0, "00+"))
> +    printf("%s{%d*0}%s\n", substr($0, 1, RSTART - 1),
> +      RLENGTH, substr($0, RSTART + RLENGTH));
> +  else print;
> +}'
> +BEGIN
> +{
> +    printf("%10000000d\n", 1);
> +    printf("%.10000000d\n", 1);
> +    printf("%10000000.10000000d\n", 1);
> +    exit(0);
> +}
> +EOF
> -- 
> 2.20.1
> 

> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel




More information about the DTrace-devel mailing list