On Sun, Aug 25, 2019 at 1:08 AM Jeff King <peff@xxxxxxxx> wrote: > > We read each line of the fast-import stream into the command_buf strbuf. > When reading a commit, we parse a line like "encoding foo" by storing a > pointer to "foo", but not making a copy. We may then read an unbounded > number of other lines (e.g., one for each modified file in the commit), > each of which writes into command_buf. > > This works out in practice for small cases, because we hand off > ownership of the heap buffer from command_buf to the cmd_hist array, and > read new commands into a fresh heap buffer. And thus the pointer to > "foo" remains valid as long as there aren't so many intermediate lines > that we end up dropping the original "encoding" line from the history. > > But as the test modification shows, if we go over our default of 100 > lines, we end up with our encoding string pointing into freed heap > memory. This seems to fail reliably by writing garbage into the output, > but running under ASan definitely detects this as a user-after-free. s/user-after-free/use-after-free/ > We can fix it by duplicating the encoding value, just as we do for other > parsed lines (e.g., an author line ends up in parse_ident, which copies > it to a new string). Eek! Thanks for fixing this up for me; patch looks good.
