From: Stefan Sperling Subject: Re: got rebase or diff library bug To: Mark Jamsek Cc: Christian Weisgerber , gameoftrees@openbsd.org Date: Sun, 4 Aug 2024 18:16:12 +0200 On Mon, Aug 05, 2024 at 01:53:22AM +1000, Mark Jamsek wrote: > Updated diff checks the buffer for the terminating newline--not the > line returned from getline(3). This fixes an OOB read in the previous > diff. ok by me too (style: could avoid the reundant == 1 in boolean context) > I still want to write a test for this case though. Great! > diffstat f997e889eb0517a24a28f2f4000d6915683fef86 b5ac15ce9816489710c3698d89d6a7ef21b4d2fe > M lib/diff3.c | 12+ 1- > > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff f997e889eb0517a24a28f2f4000d6915683fef86 b5ac15ce9816489710c3698d89d6a7ef21b4d2fe > commit - f997e889eb0517a24a28f2f4000d6915683fef86 > commit + b5ac15ce9816489710c3698d89d6a7ef21b4d2fe > blob - c1ca2fdf44d461cda6adb950263ed6a9775971ac > blob + cc7cc1ee97127908122a804cd7aa3e75540389df > --- lib/diff3.c > +++ lib/diff3.c > @@ -155,6 +155,8 @@ struct diff3_state { > > char *buf; > > + int no_eofnl; /* set if the merged file has no eof newline */ > + > BUF *diffbuf; > }; > > @@ -417,6 +419,9 @@ out: > err = got_error_from_errno("fclose"); > } > if (err == NULL && diffb) { > + if (d3s->no_eofnl == 1 && > + diffb->cb_buf[diffb->cb_len - 1] == '\n') > + --diffb->cb_len; > if (buf_write_fd(diffb, outfd) < 0) > err = got_error_from_errno("buf_write_fd"); > *overlapcnt = d3s->overlapcnt; > @@ -1063,7 +1068,13 @@ edscript(int n, struct diff3_state *d3s) > } > > if (!d3s->overlap[n]) { > - err = diff_output(d3s->diffbuf, ".\n"); > + size_t len; > + > + len = buf_len(d3s->diffbuf); > + d3s->no_eofnl = len > 0 && > + buf_getc(d3s->diffbuf, len - 1) != '\n'; > + err = diff_output(d3s->diffbuf, "%s.\n", > + d3s->no_eofnl == 1 ? "\n" : ""); > if (err) > goto done; > } else { > > > -- > Mark Jamsek > GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68 > >