Re: [PATCH v4 2/5] t5000: test tar files that overflow ustar headers

On Fri, Jul 15, 2016 at 03:37:32PM +0200, Torsten Bögershausen wrote:

quoted

So off_t is probably better. We do need to be careful, though, when
allocating objects. E.g., this:

  off_t size;
  struct git_istream *stream;
  void *buf;

  stream = open_istream(sha1, &type, &size, NULL);
  buf = xmalloc(size);
  while (1) {
	/* read stream into buf */
  }

is a security hole when size_t is less than off_t (it gets truncated in
the call to xmalloc, which allocates too few bytes). This is a toy
example, obviously, but it's something to watch out for.

That code is "illegal", it should be
 buf = xmalloc(xsize_t(size));

Sure, I agree. The point is that it is easy to forget the extra
wrapper/check, and we should be aware of it. I don't think the compiler
will warn you (probably some static analyzers would, though).

- Use the streaming interface to analyze if blobs are binary
  (That is already on my list, the old "stream and early out"
  from the olc 10/10, gmane/$293010 or so can be reused)

You might be interested in

  https://github.com/peff/git/commit/2fb07bc91f3ac6162c3dd5667d8167fc0bec6d99

I don't remember if it produced good results or not (ISTR that the cost
of setting up the streaming sometimes overwhelmed any benefit).

-Peff