git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] diff: avoid stack-buffer-read-overrun for very long name

From
MKMarcus Karlsson <mk@acc.umu.se>
Date
Apr 16, 2012, 22:27 UTC
Message-ID
<20120416222713.GA2396@moj>
In-Reply-To
<87ty0jbt5p.fsf@rho.meyering.net>
On Mon, Apr 16, 2012 at 05:20:02PM +0200, Jim Meyering wrote:
Show 36 quoted lines
> 
> Due to the use of strncpy without explicit NUL termination,
> we could end up passing names n1 or n2 that are not NUL-terminated
> to queue_diff, which requires NUL-terminated strings.
> Ensure that each is NUL terminated.
> 
> Signed-off-by: Jim Meyering <meyering@redhat.com>
> ---
> After finding strncpy problems in other projects, I audited
> git for the same and found only these two.
> 
>  diff-no-index.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 3a36144..5cd3ff5 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -109,6 +109,7 @@ static int queue_diff(struct diff_options *o,
>  				n1 = buffer1;
>  				strncpy(buffer1 + len1, p1.items[i1++].string,
>  						PATH_MAX - len1);
> +				buffer1[PATH_MAX-1] = 0;
>  			}
> 
>  			if (comp < 0)
> @@ -117,6 +118,7 @@ static int queue_diff(struct diff_options *o,
>  				n2 = buffer2;
>  				strncpy(buffer2 + len2, p2.items[i2++].string,
>  						PATH_MAX - len2);
> +				buffer2[PATH_MAX-1] = 0;
>  			}
> 
>  			ret = queue_diff(o, n1, n2);
> --
> 1.7.10.169.g146fe

Are there any guarantees that len1 and len2 does not exceed PATH_MAX? Because if there aren't any then that function looks like it could need even more improvements.

	Marcus
Previous: Jim MeyeringNext: Jim Meyering
Message 2 of 13 in “diff: avoid stack-buffer-read-overrun for very long name”
  1. diff: avoid stack-buffer-read-overrun for very long nameJim Meyering, Apr 16, 2012
  2. Marcus KarlssonApr 16, 2012
  3. Jim MeyeringApr 24, 2012
  4. Junio C HamanoApr 25, 2012
  5. Jim MeyeringApr 26, 2012
  6. Junio C HamanoApr 26, 2012
  7. Bert WesargApr 26, 2012
  8. Jim MeyeringApr 26, 2012
  9. Bert WesargApr 26, 2012
  10. Jim MeyeringApr 26, 2012
  11. Jim MeyeringApr 26, 2012
  12. Andreas EricssonApr 27, 2012
  13. Junio C HamanoApr 27, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.