<feed xmlns='http://www.w3.org/2005/Atom'>
<title>cgit/parsing.c, branch v0.10.2</title>
<subtitle>A hyperfast web frontend for git repositories written in C.</subtitle>
<id>http://git.cetero.st/cgit/atom/parsing.c?h=v0.10.2</id>
<link rel='self' href='http://git.cetero.st/cgit/atom/parsing.c?h=v0.10.2'/>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/'/>
<updated>2014-06-28T13:14:56Z</updated>
<entry>
<title>git: update for git 2.0</title>
<updated>2014-06-28T13:14:56Z</updated>
<author>
<name>Christian Hesse</name>
<email>mail@eworm.de</email>
</author>
<published>2014-05-29T15:35:46Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=79c985e13c10b498c3ea62f4607c2e2a460c3b10'/>
<id>urn:sha1:79c985e13c10b498c3ea62f4607c2e2a460c3b10</id>
<content type='text'>
prefixcmp() and suffixcmp() have been remove, functionality is now
provided by starts_with() and ends_with(). Retrurn values have been
changed, so instead of just renaming we have to fix logic.
Everything else looks just fine.
</content>
</entry>
<entry>
<title>Fix cgit_parse_url when a repo url is contained in another repo url</title>
<updated>2014-04-05T22:05:36Z</updated>
<author>
<name>Julian Maurice</name>
<email>julian.maurice@biblibre.com</email>
</author>
<published>2014-03-28T22:18:29Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=10451797fa2370aab6f4146c86e0fa939a9a982b'/>
<id>urn:sha1:10451797fa2370aab6f4146c86e0fa939a9a982b</id>
<content type='text'>
For example, if I have two repos (remove-suffix is enabled):
  /foo
  /foo/bar

http://cgit/foo/bar/ is interpreted as "repository 'foo', command 'bar'"
instead of "repository 'foo/bar'"
</content>
</entry>
<entry>
<title>parsing.c: Remove leading space from committer</title>
<updated>2014-01-15T23:53:08Z</updated>
<author>
<name>Lukas Fleischer</name>
<email>cgit@cryptocrack.de</email>
</author>
<published>2014-01-15T22:09:08Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=2a7dd4bf67edeff2c6c4f6d6d39d2d7f954d978a'/>
<id>urn:sha1:2a7dd4bf67edeff2c6c4f6d6d39d2d7f954d978a</id>
<content type='text'>
This did not really break anything in the past since spaces are ignored
when rendering HTML. Remove the preceding space anyway to prevent from
potential future problems.

Signed-off-by: Lukas Fleischer &lt;cgit@cryptocrack.de&gt;
</content>
</entry>
<entry>
<title>parsing: fix header typo</title>
<updated>2014-01-10T16:45:43Z</updated>
<author>
<name>Jason A. Donenfeld</name>
<email>Jason@zx2c4.com</email>
</author>
<published>2014-01-10T03:59:34Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=d01a6eec4355af5266b114760c7264a2e6bf73fc'/>
<id>urn:sha1:d01a6eec4355af5266b114760c7264a2e6bf73fc</id>
<content type='text'>
</content>
</entry>
<entry>
<title>Replace most uses of strncmp() with prefixcmp()</title>
<updated>2014-01-10T16:01:29Z</updated>
<author>
<name>Lukas Fleischer</name>
<email>cgit@cryptocrack.de</email>
</author>
<published>2014-01-10T11:44:35Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=36bdb2171f7154fcdf1a24d38c8ce3bd7e448cb1'/>
<id>urn:sha1:36bdb2171f7154fcdf1a24d38c8ce3bd7e448cb1</id>
<content type='text'>
This is a preparation for replacing all prefix checks with either
strip_prefix() or starts_with() when Git 1.8.6 is released.

Signed-off-by: Lukas Fleischer &lt;cgit@cryptocrack.de&gt;
</content>
</entry>
<entry>
<title>Update copyright information</title>
<updated>2014-01-08T14:10:49Z</updated>
<author>
<name>Lukas Fleischer</name>
<email>cgit@crytocrack.de</email>
</author>
<published>2014-01-08T14:10:49Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=f7f26f88755ac6a3b9af4918b51b0d6e7a692c78'/>
<id>urn:sha1:f7f26f88755ac6a3b9af4918b51b0d6e7a692c78</id>
<content type='text'>
* Name "cgit Development Team" as copyright holder to avoid listing
  every single developer.

* Update copyright ranges.

Signed-off-by: Lukas Fleischer &lt;cgit@crytocrack.de&gt;
</content>
</entry>
<entry>
<title>Mark several functions/variables static</title>
<updated>2013-03-05T00:50:39Z</updated>
<author>
<name>Lukas Fleischer</name>
<email>cgit@cryptocrack.de</email>
</author>
<published>2013-03-04T07:52:33Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=bafab423f20bc1448b293842c235965e1681f07e'/>
<id>urn:sha1:bafab423f20bc1448b293842c235965e1681f07e</id>
<content type='text'>
Spotted by parsing the output of `gcc -Wmissing-prototypes [...]`.

Signed-off-by: Lukas Fleischer &lt;cgit@cryptocrack.de&gt;
</content>
</entry>
<entry>
<title>White space around control verbs.</title>
<updated>2013-03-04T14:12:54Z</updated>
<author>
<name>Jason A. Donenfeld</name>
<email>Jason@zx2c4.com</email>
</author>
<published>2013-03-04T04:21:33Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=bdae1d8a8d39206ac75ab86f8e9ef53b2f29432e'/>
<id>urn:sha1:bdae1d8a8d39206ac75ab86f8e9ef53b2f29432e</id>
<content type='text'>
Signed-off-by: Jason A. Donenfeld &lt;Jason@zx2c4.com&gt;
</content>
</entry>
<entry>
<title>do not write outside heap buffer</title>
<updated>2012-10-02T02:03:47Z</updated>
<author>
<name>Jim Meyering</name>
<email>meyering@redhat.com</email>
</author>
<published>2012-04-23T20:06:35Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=61d4147ea2d58c9d129a084be13ccec6ae18b4d5'/>
<id>urn:sha1:61d4147ea2d58c9d129a084be13ccec6ae18b4d5</id>
<content type='text'>
* parsing.c (substr): Handle tail &lt; head.

This started when I noticed some cgit segfaults on savannah.gnu.org.
Finding the offending URL/commit and then constructing a stand-alone
reproducer were far more time-consuming than writing the actual patch.

The problem arises with a commit like this, in which the user name
part of the "Author" field is empty:

    $ git log -1
    commit 6f3f41d73393278f3ede68a2cb1e7a2a23fa3421
    Author: &lt;T at h.or&gt;
    Date:   Mon Apr 23 22:29:16 2012 +0200

Here's what happens:

(this is due to buf=malloc(0); strncpy (buf, head, -1);
 where "head" may point to plenty of attacker-specified non-NUL bytes,
 so we can overwrite a zero-length heap buffer with arbitrary data)

 Invalid write of size 1
    at 0x4A09361: strncpy (mc_replace_strmem.c:463)
    by 0x408977: substr (parsing.c:61)
    by 0x4089EF: parse_user (parsing.c:73)
    by 0x408D10: cgit_parse_commit (parsing.c:153)
    by 0x40A540: cgit_mk_refinfo (shared.c:171)
    by 0x40A581: cgit_refs_cb (shared.c:181)
    by 0x43DEB3: do_for_each_ref (refs.c:690)
    by 0x41075E: cgit_print_branches (ui-refs.c:191)
    by 0x416EF2: cgit_print_summary (ui-summary.c:56)
    by 0x40780A: summary_fn (cmd.c:120)
    by 0x40667A: process_request (cgit.c:544)
    by 0x404078: cache_process (cache.c:322)
  Address 0x4c718d0 is 0 bytes after a block of size 0 alloc'd
    at 0x4A0884D: malloc (vg_replace_malloc.c:263)
    by 0x455C85: xmalloc (wrapper.c:35)
    by 0x40894C: substr (parsing.c:60)
    by 0x4089EF: parse_user (parsing.c:73)
    by 0x408D10: cgit_parse_commit (parsing.c:153)
    by 0x40A540: cgit_mk_refinfo (shared.c:171)
    by 0x40A581: cgit_refs_cb (shared.c:181)
    by 0x43DEB3: do_for_each_ref (refs.c:690)
    by 0x41075E: cgit_print_branches (ui-refs.c:191)
    by 0x416EF2: cgit_print_summary (ui-summary.c:56)
    by 0x40780A: summary_fn (cmd.c:120)
    by 0x40667A: process_request (cgit.c:544)

 Invalid write of size 1
    at 0x4A09400: strncpy (mc_replace_strmem.c:463)
    by 0x408977: substr (parsing.c:61)
    by 0x4089EF: parse_user (parsing.c:73)
    by 0x408D10: cgit_parse_commit (parsing.c:153)
    by 0x40A540: cgit_mk_refinfo (shared.c:171)
    by 0x40A581: cgit_refs_cb (shared.c:181)
    by 0x43DEB3: do_for_each_ref (refs.c:690)
    by 0x41075E: cgit_print_branches (ui-refs.c:191)
    by 0x416EF2: cgit_print_summary (ui-summary.c:56)
    by 0x40780A: summary_fn (cmd.c:120)
    by 0x40667A: process_request (cgit.c:544)
    by 0x404078: cache_process (cache.c:322)
  Address 0x4c7192b is not stack'd, malloc'd or (recently) free'd

 Invalid write of size 1
    at 0x4A0940E: strncpy (mc_replace_strmem.c:463)
    by 0x408977: substr (parsing.c:61)
    by 0x4089EF: parse_user (parsing.c:73)
    by 0x408D10: cgit_parse_commit (parsing.c:153)
    by 0x40A540: cgit_mk_refinfo (shared.c:171)
    by 0x40A581: cgit_refs_cb (shared.c:181)
    by 0x43DEB3: do_for_each_ref (refs.c:690)
    by 0x41075E: cgit_print_branches (ui-refs.c:191)
    by 0x416EF2: cgit_print_summary (ui-summary.c:56)
    by 0x40780A: summary_fn (cmd.c:120)
    by 0x40667A: process_request (cgit.c:544)
    by 0x404078: cache_process (cache.c:322)
  Address 0x4c7192d is not stack'd, malloc'd or (recently) free'd

 Process terminating with default action of signal 11 (SIGSEGV)
  Access not within mapped region at address 0x502F000
    at 0x4A09400: strncpy (mc_replace_strmem.c:463)
    by 0x408977: substr (parsing.c:61)
    by 0x4089EF: parse_user (parsing.c:73)
    by 0x408D10: cgit_parse_commit (parsing.c:153)
    by 0x40A540: cgit_mk_refinfo (shared.c:171)
    by 0x40A581: cgit_refs_cb (shared.c:181)
    by 0x43DEB3: do_for_each_ref (refs.c:690)
    by 0x41075E: cgit_print_branches (ui-refs.c:191)
    by 0x416EF2: cgit_print_summary (ui-summary.c:56)
    by 0x40780A: summary_fn (cmd.c:120)
    by 0x40667A: process_request (cgit.c:544)
    by 0x404078: cache_process (cache.c:322)

This happens when tail - head == -1 here:
(parsing.c)

  char *substr(const char *head, const char *tail)
  {
          char *buf;

          buf = xmalloc(tail - head + 1);
          strncpy(buf, head, tail - head);
          buf[tail - head] = '\0';
          return buf;
  }

  char *parse_user(char *t, char **name, char **email, unsigned long *date)
  {
          char *p = t;
          int mode = 1;

          while (p &amp;&amp; *p) {
                  if (mode == 1 &amp;&amp; *p == '&lt;') {
                          *name = substr(t, p - 1);
                          t = p;
                          mode++;
                  } else if (mode == 1 &amp;&amp; *p == '\n') {

The fix is to handle the case of (tail &lt; head) before calling xmalloc,
thus avoiding passing an invalid value to xmalloc.

And here's the reproducer:
It was tricky to reproduce, because git prohibits use of an empty "name"
in a commit ID.  To construct the offending commit, I had to resort to
using "git hash-object".

git init -q foo &amp;&amp;
( cd foo &amp;&amp;
  echo a &gt; j &amp;&amp; git add . &amp;&amp; git ci -q --author='au &lt;T at h.or&gt;' -m. . &amp;&amp;
  h=$(git cat-file commit HEAD|sed 's/au //' \
    |git hash-object -t commit -w --stdin) &amp;&amp;
  git co -q -b test $h &amp;&amp;
  git br -q -D master &amp;&amp;
  git br -q -m test master)
git clone -q --bare foo foo.git

cat &lt;&lt;EOF &gt; in
repo.url=foo.git
repo.path=foo.git
EOF
CGIT_CONFIG=in QUERY_STRING=url=foo.git valgrind ./cgit

The valgrind output is what you see above.

AFAICS, this is not exploitable thanks (ironically) to the use of strncpy.
Since that -1 translates to SIZE_MAX and this is strncpy, not only does it
copy whatever is in "head" (up to first NUL), but it also writes
SIZE_MAX - strlen(head) NUL bytes into the destination buffer, and that
latter is guaranteed to evoke a segfault.  Since cgit is single-threaded,
AFAICS, there is no way that the buffer clobbering can be turned into
an exploit.
</content>
</entry>
<entry>
<title>Remove dead initialization in cgit_parse_commit()</title>
<updated>2011-07-22T12:21:11Z</updated>
<author>
<name>Lukas Fleischer</name>
<email>cgit@cryptocrack.de</email>
</author>
<published>2011-07-21T21:04:53Z</published>
<link rel='alternate' type='text/html' href='http://git.cetero.st/cgit/commit/?id=1e25ac5b8fe0ca8760b2786b20d36013a6197e02'/>
<id>urn:sha1:1e25ac5b8fe0ca8760b2786b20d36013a6197e02</id>
<content type='text'>
The value stored to "t" during its initialization gets overwritten in
any case, so just leave it uninitialized. Spotted by clang-analyzer.

Signed-off-by: Lukas Fleischer &lt;cgit@cryptocrack.de&gt;
Signed-off-by: Lars Hjemli &lt;hjemli@gmail.com&gt;
</content>
</entry>
</feed>
