<feed xmlns='http://www.w3.org/2005/Atom'>
<title>git/shell.c, branch jch</title>
<subtitle>Mirror of https://git.kernel.org/pub/scm/git/git.git/
</subtitle>
<id>https://git.shady.money/git/atom?h=jch</id>
<link rel='self' href='https://git.shady.money/git/atom?h=jch'/>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/'/>
<updated>2024-09-30T18:23:03Z</updated>
<entry>
<title>shell: fix leaking strings</title>
<updated>2024-09-30T18:23:03Z</updated>
<author>
<name>Patrick Steinhardt</name>
<email>ps@pks.im</email>
</author>
<published>2024-09-30T09:13:22Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=c75841687b39f4c62fba56bde08653591b9c2149'/>
<id>urn:sha1:c75841687b39f4c62fba56bde08653591b9c2149</id>
<content type='text'>
There are two memory leaks in "shell.c". The first one in `run_shell()`
is trivial and fixed without further explanation. The second one in
`cmd_main()` happens because we overwrite the `prog` variable, which
contains an allocated string. In fact though, the memory pointed to by
that variable is still in use because we use `split_cmdline()`, which
may create pointers into the middle of that string. But as we do not
have a direct pointer to the head of the allocated string anymore, we
get a complaint by the leak checker.

Address this by not overwriting the `prog` pointer.

Signed-off-by: Patrick Steinhardt &lt;ps@pks.im&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>treewide: remove unnecessary includes in source files</title>
<updated>2023-12-26T20:04:31Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2023-12-23T17:14:50Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=eea0e59ffbed6e33d171ace5be13cde9faa41639'/>
<id>urn:sha1:eea0e59ffbed6e33d171ace5be13cde9faa41639</id>
<content type='text'>
Each of these were checked with
   gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE}
to ensure that removing the direct inclusion of the header actually
resulted in that header no longer being included at all (i.e. that
no other header pulled it in transitively).

...except for a few cases where we verified that although the header
was brought in transitively, nothing from it was directly used in
that source file.  These cases were:
  * builtin/credential-cache.c
  * builtin/pull.c
  * builtin/send-pack.c

Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>treewide: remove unnecessary cache.h includes in source files</title>
<updated>2023-02-24T01:25:28Z</updated>
<author>
<name>Elijah Newren</name>
<email>newren@gmail.com</email>
</author>
<published>2023-02-24T00:09:23Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=15db4e7f4ac20cc41902a2479c7784fff8edf2e9'/>
<id>urn:sha1:15db4e7f4ac20cc41902a2479c7784fff8edf2e9</id>
<content type='text'>
We had several C files include cache.h unnecessarily.  Replace those
with an include of "git-compat-util.h" instead.  Much like the previous
commit, these have all been verified via both ensuring that
    gcc -E $SOURCE_FILE | grep '"cache.h"'
found no hits and that
    make DEVELOPER=1 ${OBJECT_FILE_FOR_SOURCE_FILE}
successfully compiles without warnings.

Signed-off-by: Elijah Newren &lt;newren@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>replace and remove run_command_v_opt()</title>
<updated>2022-10-30T18:04:51Z</updated>
<author>
<name>René Scharfe</name>
<email>l.s.r@web.de</email>
</author>
<published>2022-10-30T11:55:06Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=ddbb47fde9b6d8cd9f3728847a378f634318cfb1'/>
<id>urn:sha1:ddbb47fde9b6d8cd9f3728847a378f634318cfb1</id>
<content type='text'>
Replace the remaining calls of run_command_v_opt() with run_command()
calls and explict struct child_process variables.  This is more verbose,
but not by much overall.  The code becomes more flexible, e.g. it's easy
to extend to conditionally add a new argument.

Then remove the now unused function and its own flag names, simplifying
the run-command API.

Suggested-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: René Scharfe &lt;l.s.r@web.de&gt;
Signed-off-by: Taylor Blau &lt;me@ttaylorr.com&gt;
</content>
</entry>
<entry>
<title>Sync with 2.32.4</title>
<updated>2022-10-06T21:42:02Z</updated>
<author>
<name>Taylor Blau</name>
<email>me@ttaylorr.com</email>
</author>
<published>2022-10-06T21:42:02Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=3957f3c84e89c68e8bf3f7cb172ba6837af70506'/>
<id>urn:sha1:3957f3c84e89c68e8bf3f7cb172ba6837af70506</id>
<content type='text'>
Signed-off-by: Taylor Blau &lt;me@ttaylorr.com&gt;
</content>
</entry>
<entry>
<title>shell: limit size of interactive commands</title>
<updated>2022-10-01T04:23:38Z</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2022-09-28T22:52:48Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=71ad7fe1bcec2a115bd0ab187240348358aa7f21'/>
<id>urn:sha1:71ad7fe1bcec2a115bd0ab187240348358aa7f21</id>
<content type='text'>
When git-shell is run in interactive mode (which must be enabled by
creating $HOME/git-shell-commands), it reads commands from stdin, one
per line, and executes them.

We read the commands with git_read_line_interactively(), which uses a
strbuf under the hood. That means we'll accept an input of arbitrary
size (limited only by how much heap we can allocate). That creates two
problems:

  - the rest of the code is not prepared to handle large inputs. The
    most serious issue here is that split_cmdline() uses "int" for most
    of its types, which can lead to integer overflow and out-of-bounds
    array reads and writes. But even with that fixed, we assume that we
    can feed the command name to snprintf() (via xstrfmt()), which is
    stuck for historical reasons using "int", and causes it to fail (and
    even trigger a BUG() call).

  - since the point of git-shell is to take input from untrusted or
    semi-trusted clients, it's a mild denial-of-service. We'll allocate
    as many bytes as the client sends us (actually twice as many, since
    we immediately duplicate the buffer).

We can fix both by just limiting the amount of per-command input we're
willing to receive.

We should also fix split_cmdline(), of course, which is an accident
waiting to happen, but that can come on top. Most calls to
split_cmdline(), including the other one in git-shell, are OK because
they are reading from an OS-provided argv, which is limited in practice.
This patch should eliminate the immediate vulnerabilities.

I picked 4MB as an arbitrary limit. It's big enough that nobody should
ever run into it in practice (since the point is to run the commands via
exec, we're subject to OS limits which are typically much lower). But
it's small enough that allocating it isn't that big a deal.

The code is mostly just swapping out fgets() for the strbuf call, but we
have to add a few niceties like flushing and trimming line endings. We
could simplify things further by putting the buffer on the stack, but
4MB is probably a bit much there. Note that we'll _always_ allocate 4MB,
which for normal, non-malicious requests is more than we would before
this patch. But on the other hand, other git programs are happy to use
96MB for a delta cache. And since we'd never touch most of those pages,
on a lazy-allocating OS like Linux they won't even get allocated to
actual RAM.

The ideal would be a version of strbuf_getline() that accepted a maximum
value. But for a minimal vulnerability fix, let's keep things localized
and simple. We can always refactor further on top.

The included test fails in an obvious way with ASan or UBSan (which
notice the integer overflow and out-of-bounds reads). Without them, it
fails in a less obvious way: we may segfault, or we may try to xstrfmt()
a long string, leading to a BUG(). Either way, it fails reliably before
this patch, and passes with it. Note that we don't need an EXPENSIVE
prereq on it. It does take 10-15s to fail before this patch, but with
the new limit, we fail almost immediately (and the perl process
generating 2GB of data exits via SIGPIPE).

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Taylor Blau &lt;me@ttaylorr.com&gt;
</content>
</entry>
<entry>
<title>builtins + test helpers: use return instead of exit() in cmd_*</title>
<updated>2021-06-09T00:15:58Z</updated>
<author>
<name>Ævar Arnfjörð Bjarmason</name>
<email>avarab@gmail.com</email>
</author>
<published>2021-06-08T10:48:03Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=338abb0f045b87df5e628543800e74dec0e72cdf'/>
<id>urn:sha1:338abb0f045b87df5e628543800e74dec0e72cdf</id>
<content type='text'>
Change various cmd_* functions that claim to return an "int" to use
"return" instead of exit() to indicate an exit code. These were not
marked with NORETURN, and by directly exit()-ing we'll skip the
cleanup git.c would otherwise do (e.g. closing fd's, erroring if we
can't). See run_builtin() in git.c.

In the case of shell.c and sh-i18n--envsubst.c this was the result of
an incomplete migration to using a cmd_main() in 3f2e2297b9 (add an
extra level of indirection to main(), 2016-07-01).

This was spotted by SunCC 12.5 on Solaris 10 (gcc210 on the gccfarm).

Signed-off-by: Ævar Arnfjörð Bjarmason &lt;avarab@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>interactive: refactor code asking the user for interactive input</title>
<updated>2020-04-10T17:26:31Z</updated>
<author>
<name>Johannes Schindelin</name>
<email>johannes.schindelin@gmx.de</email>
</author>
<published>2020-04-10T11:27:50Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=08d383f23e80e418c844952fcc4e2e635962c292'/>
<id>urn:sha1:08d383f23e80e418c844952fcc4e2e635962c292</id>
<content type='text'>
There are quite a few code locations (e.g. `git clean --interactive`)
where Git asks the user for an answer. In preparation for fixing a bug
shared by all of them, and also to DRY up the code, let's refactor it.

Please note that most of these callers trimmed white-space both at the
beginning and at the end of the answer, instead of trimming only the
end (as the caller in `add-patch.c` does).

Therefore, technically speaking, we change behavior in this patch. At
the same time, it can be argued that this is actually a bug fix.

Signed-off-by: Johannes Schindelin &lt;johannes.schindelin@gmx.de&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>shell: use skip_prefix() instead of starts_with()</title>
<updated>2019-11-27T02:18:24Z</updated>
<author>
<name>René Scharfe</name>
<email>l.s.r@web.de</email>
</author>
<published>2019-11-26T15:00:43Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=ec6ee0c07a6dc93dd18003b069c78f514ccbe427'/>
<id>urn:sha1:ec6ee0c07a6dc93dd18003b069c78f514ccbe427</id>
<content type='text'>
Get rid of a magic number by using skip_prefix() instead of
starts_with().

Signed-off-by: René Scharfe &lt;l.s.r@web.de&gt;
Acked-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
</entry>
<entry>
<title>Merge branch 'nd/command-list'</title>
<updated>2018-06-01T06:06:37Z</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2018-06-01T06:06:37Z</published>
<link rel='alternate' type='text/html' href='https://git.shady.money/git/commit/?id=2289880f784326dc955f213072164539dcaf445e'/>
<id>urn:sha1:2289880f784326dc955f213072164539dcaf445e</id>
<content type='text'>
The list of commands with their various attributes were spread
across a few places in the build procedure, but it now is getting a
bit more consolidated to allow more automation.

* nd/command-list:
  completion: allow to customize the completable command list
  completion: add and use --list-cmds=alias
  completion: add and use --list-cmds=nohelpers
  Move declaration for alias.c to alias.h
  completion: reduce completable command list
  completion: let git provide the completable command list
  command-list.txt: documentation and guide line
  help: use command-list.txt for the source of guides
  help: add "-a --verbose" to list all commands with synopsis
  git: support --list-cmds=list-&lt;category&gt;
  completion: implement and use --list-cmds=main,others
  git --list-cmds: collect command list in a string_list
  git.c: convert --list-* to --list-cmds=*
  Remove common-cmds.h
  help: use command-list.h for common command list
  generate-cmds.sh: export all commands to command-list.h
  generate-cmds.sh: factor out synopsis extract code
</content>
</entry>
</feed>
