Re: [PATCH] diff: avoid stack-buffer-read-overrun for very long name
- From
- Marcus 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