gitweb bug report: hash mistaken for an option

Nicolas GeorgeMar 18, 2026, 20:22 UTC on lore

Hi.

We have a web server with gitweb (from Debian), and we observed git error messages in apache's error.log. I tracked down the issue to a request that had this:

``` ?p=…/.git;a=tree;hb=-c ```

I will not bother giving the actual URL since nobody else can check the issue in the logs or our server, but the issue can be reproduced on any server by replacing the `hb=…` parameter with `hb=-c`.

I tracked down the issue further to the `git_tree` function of the CGI script:

<https://git.kernel.org/pub/scm/git/git.git/tree/gitweb/gitweb.perl#n7200>

If `$hash_base`, i.e. the `hb` parameter, is set and not `$file_name`, then it is passed as is as the last argument of `ls-tree -z`, and since it is not a valid hash, it prints an error.

Since no shell gets invoked and the options of `ls-tree` are very limited, I do not think it counts as a security flaw. But it could become one, so better fix it.

I see really two issues:

First, that the parameter is interpreted as an option. It could become a more severe issue if new options get introduced. And it is very easy to fix: add a double dash.

Second, that the error (be it “unknown switch”, “Not a valid object name” or “not a tree object” gets written into the error log: it is an error entirely caused by the client that has no repercussion on the server, it should be either passed back to the client or ignored. This is more minor but harder to fix.

It might also be a good idea to check the rest of the source code for similar patterns.

Thanks.

Regards,

-- 
  Nicolas George

[System Info]
git version:
git version 2.39.5
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 6.1.0-44-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.164-1 (2026-03-09) x86_64
compiler info: gnuc: 12.2
libc info: glibc: 2.36
$SHELL (typically, interactive shell): /bin/zsh


[Enabled Hooks]
not run from a git repository - no hooks to show


From a52031aac02bc0704a56d8ca36ba3ee6d25d5b11 Mon Sep 17 00:00:00 2001
From: Nicolas George <george@nsup.org>
Date: Wed, 18 Mar 2026 21:11:57 +0100
Subject: [PATCH] gitweb: avoid hash being mistaken for an option

Fix (partially) weird warning in the error log of the server,
and potentially more severe issues if new options are added.

Signed-off-by: Nicolas George <george@nsup.org>
---
 gitweb/gitweb.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index fde804593b..2e131bcbe9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7217,7 +7217,7 @@ sub git_tree {
 	{
 		local $/ = "\0";
 		open my $fd, "-|", git_cmd(), "ls-tree", '-z',
-			($show_sizes ? '-l' : ()), @extra_options, $hash
+			($show_sizes ? '-l' : ()), @extra_options, "--", $hash
 			or die_error(500, "Open git-ls-tree failed");
 		@entries = map { chomp; $_ } <$fd>;
 		close $fd
-- 
2.51.0

Junio C HamanoMar 18, 2026, 21:47 UTC in reply to Nicolas George on lore

Re: gitweb bug report: hash mistaken for an option

Nicolas George <george@nsup.org> writes:

Show 22 quoted lines

> Hi.
>
> We have a web server with gitweb (from Debian), and we observed git error
> messages in apache's error.log. I tracked down the issue to a request that
> had this:
>
> ```
> ?p=…/.git;a=tree;hb=-c
> ```
>
> I will not bother giving the actual URL since nobody else can check the
> issue in the logs or our server, but the issue can be reproduced on any
> server by replacing the `hb=…` parameter with `hb=-c`.
>
> I tracked down the issue further to the `git_tree` function of the CGI
> script:
>
> <https://git.kernel.org/pub/scm/git/git.git/tree/gitweb/gitweb.perl#n7200>
>
> If `$hash_base`, i.e. the `hb` parameter, is set and not `$file_name`, then
> it is passed as is as the last argument of `ls-tree -z`, and since it is not
> a valid hash, it prints an error.

In other words, garbage-in garbage-out?

FWIW, this also gives an error message

    $ git ls-tree no-such-tree
    fatal: Not a valid object name no-such

and even though it may give slightly a smaller error message than "git ls-tree -c", it would not stay silent. So I am not sure if there is anything to fix here, short of redirecting your standard error to /dev/null or something.

Show 5 quoted lines

> Second, that the error (be it “unknown switch”, “Not a valid object name” or
> “not a tree object” gets written into the error log: it is an error entirely
> caused by the client that has no repercussion on the server, it should be
> either passed back to the client or ignored. This is more minor but harder
> to fix.

For that, you'd need to capture standard error stream and relay it to the user, I guess. That does sound like the right fix to deal with any wrong input that comes in the web request.

This is a tangent, but I actually think "ls-tree -- $whatever" that forces $whatever to be interpreted as the tree object name _is_ a bug. A double-dash should signal end of revisions and beginning of the pathspec. We may want to fix it in "git ls-tree", regardless of any gitweb issue.

While I do not think it is a fix to force the end-user supplied parameter to be interpreted as a tree object name, if we wanted to do so in durable way that won't be broken even after we correct the double-dash bug in ls-tree, we probably should write

    $ git ls-tree -z --end-of-options $hb

instead.

Thanks.