| Commit message (Collapse) | Author |
|---|
|
* 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: <T at h.or>
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 <T at h.or>' -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 <<EOF > 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.
|
|
Since cgit snapshots of tags are often used for releases, we don't
want the rarely used feature of the gzip compressor that includes
an embedded timestamp into the archive, since this makes each tarball
of the same (potentially signed) tag different.
This commit refactors the archive handling code a bit so that each
different format is able to run with an arbitrary argv for the filter.
|
|
|
|
Add two options, one for doing the ordinary name sorts in a
case-insensitive manner, and another for choosing to sort repos in each
section by age instead of by name.
|
|
Use gitweb.category from git config to determine repo's section, if
option is enabled.
|
|
Use gitweb.description instead of description file to determine
description, if option is enabled.
|
|
Don't bother with 'body' and 'div#cgit form', since
everything is wrapped in 'div#cgit' already.
Removing these two types makes embedding even easier.
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
improves readability when embedding into a page that
has the text color set to a different color
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
When embedding cgit in other pages, the logo alignment needs to be
specified to avoid any css rules from the embedding page to make the
page look bad.
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
to facilitate easier embedding
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
|
|
|
|
v2: incorporate remarks of Lukas
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
v2: incorporate remarks of Lukas
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
|
|
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
ctx.qry.head can be NULL in some cases due to bad requests
by weird bots. I managed to reproduce with:
PATH_INFO=/repo.git/shop.php QUERY_STRING=id=
Signed-off-by: Eric Wong <normalperson@yhbt.net>
|
|
**L would have worked well too. Depending on the distribution sizeof *L
may return 8 instead of 4. **L is preferable, but since we don't expect
this datatype to change very often, sizeof int is less subtle and easier
to understand.
Signed-off-by: Jamie Couture <jamie.couture@gmail.com>
|
|
Each individual string may be too long for its respective
dimension of the LCS table.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
|
|
When a repository is empty, the ATOM feed link is written in the header,
but this involves formatting ctx->qry.head which is NULL in this case.
With glibc, vsnprintf formats "%s" with a NULL input as "(null)" but on
Solaris this results in a segmentation fault. Since we don't have a
meaningful head for the atom feed in an empty repository, it's simplest
not to write out the link element at all.
Signed-off-by: John Keeping <john@metanate.com>
|
|
The git tarballs are currently not available from kernel.org, so for now
the makefile will download autogenerated tarballs from cgit.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
|
|
When side-by-side-diffs=1 was set in cgitrc, specifying 'ss=0' in the
querystring would not set the 'unified' option as active in the dropdown
box used to select diffmode.
|
|
When side-by-side-diffs=1 was set in cgitrc, specyfing 'ss=0' in the query-
string would not switch to unified diffs. This patch fixes the issue by
introducing a separate variable to track the occurrence of "ss" in the
querystring.
|
|
If remote branches are not enabled, the branches are still listed in
the log view. This patch removes them if enable-remote-branches=0.
|
|
When the repolist is paged, the page-links are missing the sort parameter,
causing the initial page to be custom sorted, but any clicked page will
then be with the default sort order again.
|
|
Printing deferred line changes for files containing long lines would
cause a segfault.
- limit LCS table size: 128x128.
- move LCS table to global context: avoid allocating/freeing memory
for every deferred line change.
Signed-off-by: Jamie Couture <jamie.couture@gmail.com>
|
|
Some setenv() implementations (e.g. the one in OpenBSD's stdlib)
segfault if we pass a NULL value. Only set environment variables if the
corresponding settings are defined to avoid this.
Note that this is a minor behaviour change as environment variables were
supposed to be set to an empty string if a setting was undefined. Given
that this feature isn't part of any official release yet, there's no
need to worry about backwards compatibility, really. Change the
documentation accordingly.
Signed-off-by: Lukas Fleischer <cgit@cryptocrack.de>
|
|
This isn't used anywhere and prevents the code from being compiled on
other platforms, such as *BSD.
Signed-off-by: Lukas Fleischer <cgit@cryptocrack.de>
|
|
The file name displayed in the rename hint should be escaped to avoid
XSS. Note that this vulnerability is only applicable when an attacker
has gained push access to the repository.
Signed-off-by: Lukas Fleischer <cgit@cryptocrack.de>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
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 <cgit@cryptocrack.de>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
This fixes a segfault for me with with -O2 optimization on x86
with gcc (Debian 4.4.5-8) 4.4.5
I can reliably reproduce it with the following parameters
when pointed to the git.git repository:
PATH_INFO='/git-core.git/diff/'
QUERY_STRING='id=2b93bfac0f5bcabbf60f174f4e7bfa9e318e64d5&id2=d6da71a9d16b8cf27f9d8f90692d3625c849cbc8'
Signed-off-by: Eric Wong <normalperson@yhbt.net>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
Some tests would otherwise fail because commands such as
cd trash/repos/foo && git rev-list --reverse HEAD | head -1
would return 2 lines instead of 1: the 'cd' command also
prints the path when CDPATH is set.
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
When looking for the modtime of a repo we used to rely on repo.defbranch
having a value. This is no longer true so this patch provides a default
value when needed.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
For sites that do not want to configure mime types by hand but
still want the correct mime type for 'plain' blobs, configuring
a mime type file is made possible. This is handy since such a
file is normally already provided (at least on Linux systems).
Also, this reflects the gitweb option '$mimetypes_file'
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The default length for sha1 abbreviations in git is 7.
A '#num' at the beginning of the commit message is now
recognised, a ':#num' as well, etc.: a '#num' anywhere
is now converted to a link.
Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The resolve_ref() function handles reading of git- and filesystem
symbolic links (including proper whitespace trimming) and packed refs.
There's no point in reimplementing this function in cgit.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
There's no need to invoke guess_defbranch() for each repo during
scan-path, since repo.defbranch is only used when repo content is
being displayed.
Also, some users prefer to register their projects manually in cgitrc
but they got no benefit from the new repo.defbranch handling.
This patch tries to rectify these issues by only invoking guess_defbranch()
when needed, regardless of how the repo was registered.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
This is a saner alternative than hardcoding the default branch to be
"master". The add_repo() function will now check for a symbolic ref in
repo_path/HEAD. If there is a suitable one, overwrite repo->defbranch
with it. Note that you'll need to strip the newline from the file (->
len-17).
If HEAD is a symbolic link pointing directly to a branch below
refs/heads/, do a readlink() instead to find the ref name.
Signed-off-by: Julius Plenz <plenz@cis.fu-berlin.de>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The current 'Not a git repository' error message is not very helpful,
since it doesn't state the cause of the problem.
This patch uses errno to provide a hint of the underlying problem. It
would have been even better to give the exact cause (e.g. for ENOENT it
would be nice to know which file/directory is missing), but that would
require reimplementing setup_git_directory_gently() which seems a bit
overkill.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The old default value was an abomination which never should have been
allowed to see the light of day.
This patch removes the default, which is a backwards incompatible change
with low probability of causing anyone any real trouble (a repo with
submodules, displayed by cgit using the default value of `module-link`,
is very unlikely to actually generate working links).
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
In ui-plain.c, the links generated for submodule entry ignored the fact
that the entry was in fact a submodule. This patch adds proper submodule
links to the plain directory listings.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The current 'repo.module-link' option is sufficient when all gitlinks
in a repository can be converted to commit links in a uniform way, but
not when different submodules/paths needs different settings.
This patch adds support for 'repo.module-link.<path>', which will be
used for linking to submodules at paths matching one such entry.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The global module-link option can be overridden per repo, but this has
never been documented.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
The current 'clone-prefix' setting has some known issues:
* All repos get the same 'clone-prefix' value since the setting is not
adopted during repo registration (in cgitrc, or during scan-path traversal),
but only when the setting is used.
* The generated clone-urls for a repo is a combination of 'clone-prefix', a
slash and the repo url. This doesn't work well with e.g. ssh-style urls
like 'git@example.org:repo.git', since the inserted slash will make the
repo relative to the filesystem root.
* If 'remove-suffix' is enabled, the generated clone-urls will not work for
cloning (except for http-urls to cgit itself) since they miss the '.git'
suffix.
The new 'clone-url' setting is designed to avoid the mentioned issues:
* Each repo adopts the default 'clone-url' when the repo is defined. This
allows different groups of repos to adopt different values.
* The clone-urls for a repo is generated by expanding environment variables
in a string template without inserting arbitrary characters, hence any
kind of clone-url can be generated.
* Macro expansion also eases the 'remove-suffix' pain since it's now
possible to define e.g. 'clone-url=git://foo.org/$CGIT_REPO_URL.git' for
a set of repos. A furter improvement would be to define e.g.
$CGIT_REPO_SUFFIX to '.git' for all repos which had their url prettified,
or to store the original $CGIT_REPO_URL in e.g. $CGIT_REPO_REAL_URL before
suffix removal.
Reviewed-by: Ferry Huberts <mailings@hupie.com>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
When cgit learned to setup environment variables for certain repo
settings before invoking a filter process, the setup occurred inside
cgit_open_filter().
This patch moves the setup out of cgit_open_filter() and into
prepare_repo_cmd() to prepare for additional uses of these variables.
Reviewed-by: Ferry Huberts <mailings@hupie.com>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
This patch makes the generated man-page for the filer api section more
similar to the other sections. Also, the bulleted list of environment
variables wasn't rendered correctly (with asciidoc 8.5.2), without an
empty line before the first item.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
This is a new feature in cgit-0.9 which was formerly undocumented.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|
|
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
|