From babf94e04e74123eb658a823213c062663cdadd6 Mon Sep 17 00:00:00 2001 From: Jason A. Donenfeld Date: Sat, 25 May 2013 17:47:15 +0000 Subject: ui-summary: Disallow directory traversal Using the url= query string, it was possible request arbitrary files from the filesystem if the readme for a given page was set to a filesystem file. The following request would return my /etc/passwd file: http://git.zx2c4.com/?url=/somerepo/about/../../../../etc/passwd http://data.zx2c4.com/cgit-directory-traversal.png This fix uses realpath(3) to canonicalize all paths, and then compares the base components. This fix introduces a subtle timing attack, whereby a client can check whether or not strstr is called using timing measurements in order to determine if a given file exists on the filesystem. This fix also does not account for filesystem race conditions (TOCTOU) in resolving symlinks. Signed-off-by: Jason A. Donenfeld --- --- ui-summary.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) --- a/ui-summary.c +++ b/ui-summary.c @@ -96,6 +96,7 @@ void cgit_print_repo_readme(char *path) * to the directory containing the configured readme. */ if (path) { + char *resolved_base, *resolved_full; slash = strrchr(ctx.repo->readme, '/'); if (!slash) { if (!colon) @@ -104,7 +105,19 @@ void cgit_print_repo_readme(char *path) } tmp = xmalloc(slash - ctx.repo->readme + 1 + strlen(path) + 1); strncpy(tmp, ctx.repo->readme, slash - ctx.repo->readme + 1); + if (!ref) + resolved_base = realpath(tmp, NULL); strcpy(tmp + (slash - ctx.repo->readme + 1), path); + if (!ref) { + resolved_full = realpath(tmp, NULL); + if (!resolved_base || !resolved_full || + strstr(resolved_full, resolved_base) != resolved_full) { + free(tmp); + return; + } + free(resolved_base); + free(resolved_full); + } } else tmp = ctx.repo->readme;