Revision 7b70e9efb18c2cc3f219af399bd384c5801ba1d7 authored by Jeff King on 16 April 2024, 08:35:33 UTC, committed by Johannes Schindelin on 17 April 2024, 20:29:56 UTC
The upload-pack command tries to avoid trusting the repository in which it's run (e.g., by not running any hooks and not using any config that contains arbitrary commands). But if the server side of a fetch or a clone is a partial clone, then either upload-pack or its child pack-objects may run a lazy "git fetch" under the hood. And it is very easy to convince fetch to run arbitrary commands. The "server" side can be a local repository owned by someone else, who would be able to configure commands that are run during a clone with the current user's permissions. This issue has been designated CVE-2024-32004. The fix in this commit's parent helps in this scenario, as well as in related scenarios using SSH to clone, where the untrusted .git directory is owned by a different user id. But if you received one as a zip file, on a USB stick, etc, it may be owned by your user but still untrusted. This has been designated CVE-2024-32465. To mitigate the issue more completely, let's disable lazy fetching entirely during `upload-pack`. While fetching from a partial repository should be relatively rare, it is certainly not an unreasonable workflow. And thus we need to provide an escape hatch. This commit works by respecting a GIT_NO_LAZY_FETCH environment variable (to skip the lazy-fetch), and setting it in upload-pack, but only when the user has not already done so (which gives us the escape hatch). The name of the variable is specifically chosen to match what has already been added in 'master' via e6d5479e7a (git: extend --no-lazy-fetch to work across subprocesses, 2024-02-27). Since we're building this fix as a backport for older versions, we could cherry-pick that patch and its earlier steps. However, we don't really need the niceties (like a "--no-lazy-fetch" option) that it offers. By using the same name, everything should just work when the two are eventually merged, but here are a few notes: - the blocking of the fetch in e6d5479e7a is incomplete! It sets fetch_if_missing to 0 when we setup the repository variable, but that isn't enough. pack-objects in particular will call prefetch_to_pack() even if that variable is 0. This patch by contrast checks the environment variable at the lowest level before we call the lazy fetch, where we can be sure to catch all code paths. Possibly the setting of fetch_if_missing from e6d5479e7a can be reverted, but it may be useful to have. For example, some code may want to use that flag to change behavior before it gets to the point of trying to start the fetch. At any rate, that's all outside the scope of this patch. - there's documentation for GIT_NO_LAZY_FETCH in e6d5479e7a. We can live without that here, because for the most part the user shouldn't need to set it themselves. The exception is if they do want to override upload-pack's default, and that requires a separate documentation section (which is added here) - it would be nice to use the NO_LAZY_FETCH_ENVIRONMENT macro added by e6d5479e7a, but those definitions have moved from cache.h to environment.h between 2.39.3 and master. I just used the raw string literals, and we can replace them with the macro once this topic is merged to master. At least with respect to CVE-2024-32004, this does render this commit's parent commit somewhat redundant. However, it is worth retaining that commit as defense in depth, and because it may help other issues (e.g., symlink/hardlink TOCTOU races, where zip files are not really an interesting attack vector). The tests in t0411 still pass, but now we have _two_ mechanisms ensuring that the evil command is not run. Let's beef up the existing ones to check that they failed for the expected reason, that we refused to run upload-pack at all with an alternate user id. And add two new ones for the same-user case that both the restriction and its escape hatch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent f4aa8c8
trace.h
#ifndef TRACE_H
#define TRACE_H
#include "git-compat-util.h"
#include "strbuf.h"
/**
* The trace API can be used to print debug messages to stderr or a file. Trace
* code is inactive unless explicitly enabled by setting `GIT_TRACE*` environment
* variables.
*
* The trace implementation automatically adds `timestamp file:line ... \n` to
* all trace messages. E.g.:
*
* ------------
* 23:59:59.123456 git.c:312 trace: built-in: git 'foo'
* 00:00:00.000001 builtin/foo.c:99 foo: some message
* ------------
*
* Bugs & Caveats
* --------------
*
* GIT_TRACE_* environment variables can be used to tell Git to show
* trace output to its standard error stream. Git can often spawn a pager
* internally to run its subcommand and send its standard output and
* standard error to it.
*
* Because GIT_TRACE_PERFORMANCE trace is generated only at the very end
* of the program with atexit(), which happens after the pager exits, it
* would not work well if you send its log to the standard error output
* and let Git spawn the pager at the same time.
*
* As a work around, you can for example use '--no-pager', or set
* GIT_TRACE_PERFORMANCE to another file descriptor which is redirected
* to stderr, or set GIT_TRACE_PERFORMANCE to a file specified by its
* absolute path.
*
* For example instead of the following command which by default may not
* print any performance information:
*
* ------------
* GIT_TRACE_PERFORMANCE=2 git log -1
* ------------
*
* you may want to use:
*
* ------------
* GIT_TRACE_PERFORMANCE=2 git --no-pager log -1
* ------------
*
* or:
*
* ------------
* GIT_TRACE_PERFORMANCE=3 3>&2 git log -1
* ------------
*
* or:
*
* ------------
* GIT_TRACE_PERFORMANCE=/path/to/log/file git log -1
* ------------
*
*/
/**
* Defines a trace key (or category). The default (for API functions that
* don't take a key) is `GIT_TRACE`.
*
* E.g. to define a trace key controlled by environment variable `GIT_TRACE_FOO`:
*
* ------------
* static struct trace_key trace_foo = TRACE_KEY_INIT(FOO);
*
* static void trace_print_foo(const char *message)
* {
* trace_printf_key(&trace_foo, "%s", message);
* }
* ------------
*
* Note: don't use `const` as the trace implementation stores internal state in
* the `trace_key` structure.
*/
struct trace_key {
const char * const key;
int fd;
unsigned int initialized : 1;
unsigned int need_close : 1;
};
extern struct trace_key trace_default_key;
#define TRACE_KEY_INIT(name) { .key = "GIT_TRACE_" #name }
extern struct trace_key trace_perf_key;
extern struct trace_key trace_setup_key;
void trace_repo_setup(const char *prefix);
/**
* Checks whether the trace key is enabled. Used to prevent expensive
* string formatting before calling one of the printing APIs.
*/
int trace_want(struct trace_key *key);
/**
* Enables or disables tracing for the specified key, as if the environment
* variable was set to the given value.
*/
void trace_override_envvar(struct trace_key *key, const char *value);
/**
* Disables tracing for the specified key, even if the environment variable
* was set.
*/
void trace_disable(struct trace_key *key);
/**
* Returns nanoseconds since the epoch (01/01/1970), typically used
* for performance measurements.
* Currently there are high precision timer implementations for Linux (using
* `clock_gettime(CLOCK_MONOTONIC)`) and Windows (`QueryPerformanceCounter`).
* Other platforms use `gettimeofday` as time source.
*/
uint64_t getnanotime(void);
void trace_command_performance(const char **argv);
void trace_verbatim(struct trace_key *key, const void *buf, unsigned len);
uint64_t trace_performance_enter(void);
/*
* TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The
* default is __FILE__, as it is consistent with assert(), and static function
* names are not necessarily unique.
*
* __FILE__ ":" __FUNCTION__ doesn't work with GNUC, as __FILE__ is supplied
* by the preprocessor as a string literal, and __FUNCTION__ is filled in by
* the compiler as a string constant.
*/
#ifndef TRACE_CONTEXT
# define TRACE_CONTEXT __FILE__
#endif
/**
* Macros to add the file:line of the calling code, instead of that of
* the trace function itself.
*
* Note: with C99 variadic macros, __VA_ARGS__ must include the last fixed
* parameter ('format' in this case). Otherwise, a call without variable
* arguments will have a surplus ','. E.g.:
*
* #define foo(format, ...) bar(format, __VA_ARGS__)
* foo("test");
*
* will expand to
*
* bar("test",);
*
* which is invalid (note the ',)'). With GNUC, '##__VA_ARGS__' drops the
* comma, but this is non-standard.
*/
/**
* trace_printf(), accepts "const char *format, ...".
*
* Prints a formatted message, similar to printf.
*/
#define trace_printf(...) trace_printf_key(&trace_default_key, __VA_ARGS__)
/**
* trace_printf_key(), accepts "struct trace_key *key, const char *format, ...".
*/
#define trace_printf_key(key, ...) \
do { \
if (trace_pass_fl(key)) \
trace_printf_key_fl(TRACE_CONTEXT, __LINE__, key, \
__VA_ARGS__); \
} while (0)
/**
* trace_argv_printf(), accepts "struct trace_key *key, const char *format, ...)".
*
* Prints a formatted message, followed by a quoted list of arguments.
*/
#define trace_argv_printf(argv, ...) \
do { \
if (trace_pass_fl(&trace_default_key)) \
trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, \
argv, __VA_ARGS__); \
} while (0)
/**
* trace_strbuf(), accepts "struct trace_key *key, const struct strbuf *data".
*
* Prints the strbuf, without additional formatting (i.e. doesn't
* choke on `%` or even `\0`).
*/
#define trace_strbuf(key, data) \
do { \
if (trace_pass_fl(key)) \
trace_strbuf_fl(TRACE_CONTEXT, __LINE__, key, data);\
} while (0)
/**
* trace_performance(), accepts "uint64_t nanos, const char *format, ...".
*
* Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled.
*
* Example:
* ------------
* uint64_t t = 0;
* for (;;) {
* // ignore
* t -= getnanotime();
* // code section to measure
* t += getnanotime();
* // ignore
* }
* trace_performance(t, "frotz");
* ------------
*/
#define trace_performance(nanos, ...) \
do { \
if (trace_pass_fl(&trace_perf_key)) \
trace_performance_fl(TRACE_CONTEXT, __LINE__, nanos,\
__VA_ARGS__); \
} while (0)
/**
* trace_performance_since(), accepts "uint64_t start, const char *format, ...".
*
* Prints elapsed time since 'start' if GIT_TRACE_PERFORMANCE is enabled.
*
* Example:
* ------------
* uint64_t start = getnanotime();
* // code section to measure
* trace_performance_since(start, "foobar");
* ------------
*/
#define trace_performance_since(start, ...) \
do { \
if (trace_pass_fl(&trace_perf_key)) \
trace_performance_fl(TRACE_CONTEXT, __LINE__, \
getnanotime() - (start), \
__VA_ARGS__); \
} while (0)
/**
* trace_performance_leave(), accepts "const char *format, ...".
*/
#define trace_performance_leave(...) \
do { \
if (trace_pass_fl(&trace_perf_key)) \
trace_performance_leave_fl(TRACE_CONTEXT, __LINE__, \
getnanotime(), \
__VA_ARGS__); \
} while (0)
/* backend functions, use non-*fl macros instead */
__attribute__((format (printf, 4, 5)))
void trace_printf_key_fl(const char *file, int line, struct trace_key *key,
const char *format, ...);
__attribute__((format (printf, 4, 5)))
void trace_argv_printf_fl(const char *file, int line, const char **argv,
const char *format, ...);
void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
const struct strbuf *data);
__attribute__((format (printf, 4, 5)))
void trace_performance_fl(const char *file, int line,
uint64_t nanos, const char *fmt, ...);
__attribute__((format (printf, 4, 5)))
void trace_performance_leave_fl(const char *file, int line,
uint64_t nanos, const char *fmt, ...);
static inline int trace_pass_fl(struct trace_key *key)
{
return key->fd || !key->initialized;
}
#endif /* TRACE_H */
![swh spinner](/static/img/swh-spinner.gif)
Computing file changes ...