parsing: fix header typo

2014-01-10T16:45:43Z

Replace most uses of strncmp() with prefixcmp()

2014-01-10T16:01:29Z

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

Update copyright information

2014-01-08T14:10:49Z

* Name "cgit Development Team" as copyright holder to avoid listing every single developer. * Update copyright ranges. Signed-off-by: Lukas Fleischer

Mark several functions/variables static

2013-03-05T00:50:39Z

Spotted by parsing the output of `gcc -Wmissing-prototypes [...]`. Signed-off-by: Lukas Fleischer

White space around control verbs.

2013-03-04T14:12:54Z

Signed-off-by: Jason A. Donenfeld

do not write outside heap buffer

2012-10-02T02:03:47Z

* parsing.c (substr): Handle tail < 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: 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 && *p) { if (mode == 1 && *p == '<') { *name = substr(t, p - 1); t = p; mode++; } else if (mode == 1 && *p == '\n') { The fix is to handle the case of (tail < 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 && ( cd foo && echo a > j && git add . && git ci -q --author='au ' -m. . && h=$(git cat-file commit HEAD|sed 's/au //' \ |git hash-object -t commit -w --stdin) && git co -q -b test $h && git br -q -D master && git br -q -m test master) git clone -q --bare foo foo.git cat < 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.

Remove dead initialization in cgit_parse_commit()

2011-07-22T12:21:11Z

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 Signed-off-by: Lars Hjemli

Avoid null pointer dereference in reencode().

2011-05-23T20:58:35Z

Returning "*txt" if "txt" is a null pointer is a bad thing. Spotted with clang-analyzer. Signed-off-by: Lukas Fleischer Signed-off-by: Lars Hjemli

fix two encoding bugs

2011-03-26T14:21:07Z

reencode() takes three arguments in the order (txt, from, to), opposed to reencode_string, which will, like iconv, handle the arguments with from and to swapped. Fix that (this makes reencode more intuitive). If src and dst encoding are equivalent, don't do any encoding. If no special encoding parameter is found within the commit, assume UTF-8 and explicitly convert to PAGE_ENCODING. The change to reencode() mentioned above avoids re-encoding a UTF-8 string to UTF-8, for example. Signed-off-by: Julius Plenz Signed-off-by: Lars Hjemli

Reencode author and committer

2010-07-13T17:24:55Z

When a commit has a specific encoding, this encoding also applies to the author and committer name and email. Signed-off-by: Lars Hjemli

cgit/parsing.c, branch lf/filter