diff options
author | Jason A. Donenfeld | 2013-05-25 19:47:15 +0200 |
---|---|---|
committer | Jason A. Donenfeld | 2013-05-25 20:33:28 +0200 |
commit | fe36f84d843cd755c6dab629a0758264de5bcc00 (patch) | |
tree | fee8af2ed0f3df2fa9015453ce3e8d721df6a0cd | |
parent | cgitrc.5: information on directory traversal and multiple readme files (diff) | |
download | cgit-fe36f84d843cd755c6dab629a0758264de5bcc00.tar.gz cgit-fe36f84d843cd755c6dab629a0758264de5bcc00.zip |
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 <Jason@zx2c4.com>
-rw-r--r-- | ui-summary.c | 16 |
1 files changed, 16 insertions, 0 deletions
diff --git a/ui-summary.c b/ui-summary.c index 2f8a822..57206dd 100644 --- a/ui-summary.c +++ b/ui-summary.c | |||
@@ -99,6 +99,7 @@ void cgit_print_summary() | |||
99 | void cgit_parse_readme(const char *readme, const char *path, char **filename, char **ref, struct cgit_repo *repo) | 99 | void cgit_parse_readme(const char *readme, const char *path, char **filename, char **ref, struct cgit_repo *repo) |
100 | { | 100 | { |
101 | const char *slash, *colon; | 101 | const char *slash, *colon; |
102 | char *resolved_base, *resolved_full; | ||
102 | 103 | ||
103 | *filename = NULL; | 104 | *filename = NULL; |
104 | *ref = NULL; | 105 | *ref = NULL; |
@@ -133,7 +134,19 @@ void cgit_parse_readme(const char *readme, const char *path, char **filename, ch | |||
133 | } | 134 | } |
134 | *filename = xmalloc(slash - readme + 1 + strlen(path) + 1); | 135 | *filename = xmalloc(slash - readme + 1 + strlen(path) + 1); |
135 | strncpy(*filename, readme, slash - readme + 1); | 136 | strncpy(*filename, readme, slash - readme + 1); |
137 | if (!(*ref)) | ||
138 | resolved_base = realpath(*filename, NULL); | ||
136 | strcpy(*filename + (slash - readme + 1), path); | 139 | strcpy(*filename + (slash - readme + 1), path); |
140 | if (!(*ref)) | ||
141 | resolved_full = realpath(*filename, NULL); | ||
142 | if (!(*ref) && (!resolved_base || !resolved_full || strstr(resolved_full, resolved_base) != resolved_full)) { | ||
143 | free(*filename); | ||
144 | *filename = NULL; | ||
145 | } | ||
146 | if (!(*ref)) { | ||
147 | free(resolved_base); | ||
148 | free(resolved_full); | ||
149 | } | ||
137 | } else | 150 | } else |
138 | *filename = xstrdup(readme); | 151 | *filename = xstrdup(readme); |
139 | } | 152 | } |
@@ -143,6 +156,9 @@ void cgit_print_repo_readme(char *path) | |||
143 | char *filename, *ref; | 156 | char *filename, *ref; |
144 | cgit_parse_readme(ctx.repo->readme, path, &filename, &ref, ctx.repo); | 157 | cgit_parse_readme(ctx.repo->readme, path, &filename, &ref, ctx.repo); |
145 | 158 | ||
159 | if (!filename) | ||
160 | return; | ||
161 | |||
146 | /* Print the calculated readme, either from the git repo or from the | 162 | /* Print the calculated readme, either from the git repo or from the |
147 | * filesystem, while applying the about-filter. | 163 | * filesystem, while applying the about-filter. |
148 | */ | 164 | */ |