Commit 2288a713e00357107cb1fe6cc9a0c518e0e2d575
2020-02-07T12:15:34
repository: check error codes when reading common link When checking whether a path is a valid repository path, we try to read the "commondir" link file. In the process, we neither confirm that constructing the file's path succeeded nor do we verify that reading the file succeeded, which might cause us to verify repositories on an empty or bogus path later on. Fix this by checking return values. As the function to verify repos doesn't currently support returning errors, this commit also refactors the function to return an error code, passing validity of the repo via an out parameter instead, and adjusts all existing callers.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240
diff --git a/src/repository.c b/src/repository.c
index 2469e13..fe0d696 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -189,19 +189,25 @@ void git_repository_free(git_repository *repo)
*
* Open a repository object from its path
*/
-static bool valid_repository_path(git_buf *repository_path, git_buf *common_path)
+static int is_valid_repository_path(bool *out, git_buf *repository_path, git_buf *common_path)
{
+ int error;
+
+ *out = false;
+
/* Check if we have a separate commondir (e.g. we have a
* worktree) */
if (git_path_contains_file(repository_path, GIT_COMMONDIR_FILE)) {
git_buf common_link = GIT_BUF_INIT;
- git_buf_joinpath(&common_link, repository_path->ptr, GIT_COMMONDIR_FILE);
- git_futils_readbuffer(&common_link, common_link.ptr);
- git_buf_rtrim(&common_link);
+ if ((error = git_buf_joinpath(&common_link, repository_path->ptr, GIT_COMMONDIR_FILE)) < 0 ||
+ (error = git_futils_readbuffer(&common_link, common_link.ptr)) < 0)
+ return error;
+ git_buf_rtrim(&common_link);
if (git_path_is_relative(common_link.ptr)) {
- git_buf_joinpath(common_path, repository_path->ptr, common_link.ptr);
+ if ((error = git_buf_joinpath(common_path, repository_path->ptr, common_link.ptr)) < 0)
+ return error;
} else {
git_buf_swap(common_path, &common_link);
}
@@ -209,24 +215,26 @@ static bool valid_repository_path(git_buf *repository_path, git_buf *common_path
git_buf_dispose(&common_link);
}
else {
- git_buf_set(common_path, repository_path->ptr, repository_path->size);
+ if ((error = git_buf_set(common_path, repository_path->ptr, repository_path->size)) < 0)
+ return error;
}
/* Make sure the commondir path always has a trailing * slash */
if (git_buf_rfind(common_path, '/') != (ssize_t)common_path->size - 1)
- git_buf_putc(common_path, '/');
+ if ((error = git_buf_putc(common_path, '/')) < 0)
+ return error;
/* Ensure HEAD file exists */
if (git_path_contains_file(repository_path, GIT_HEAD_FILE) == false)
- return false;
-
+ return 0;
/* Check files in common dir */
if (git_path_contains_dir(common_path, GIT_OBJECTS_DIR) == false)
- return false;
+ return 0;
if (git_path_contains_dir(common_path, GIT_REFS_DIR) == false)
- return false;
+ return 0;
- return true;
+ *out = true;
+ return 0;
}
static git_repository *repository_alloc(void)
@@ -441,15 +449,15 @@ static int find_repo(
uint32_t flags,
const char *ceiling_dirs)
{
- int error;
git_buf path = GIT_BUF_INIT;
git_buf repo_link = GIT_BUF_INIT;
git_buf common_link = GIT_BUF_INIT;
struct stat st;
dev_t initial_device = 0;
int min_iterations;
- bool in_dot_git;
+ bool in_dot_git, is_valid;
size_t ceiling_offset = 0;
+ int error;
git_buf_clear(gitdir_path);
@@ -475,9 +483,8 @@ static int find_repo(
for (;;) {
if (!(flags & GIT_REPOSITORY_OPEN_NO_DOTGIT)) {
if (!in_dot_git) {
- error = git_buf_joinpath(&path, path.ptr, DOT_GIT);
- if (error < 0)
- break;
+ if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0)
+ goto out;
}
in_dot_git = !in_dot_git;
}
@@ -491,28 +498,33 @@ static int find_repo(
break;
if (S_ISDIR(st.st_mode)) {
- if (valid_repository_path(&path, &common_link)) {
- git_path_to_dir(&path);
- git_buf_set(gitdir_path, path.ptr, path.size);
+ if ((error = is_valid_repository_path(&is_valid, &path, &common_link)) < 0)
+ goto out;
+
+ if (is_valid) {
+ if ((error = git_path_to_dir(&path)) < 0 ||
+ (error = git_buf_set(gitdir_path, path.ptr, path.size)) < 0)
+ goto out;
if (gitlink_path)
- git_buf_attach(gitlink_path,
- git_worktree__read_link(path.ptr, GIT_GITDIR_FILE), 0);
+ if ((error = git_buf_attach(gitlink_path, git_worktree__read_link(path.ptr, GIT_GITDIR_FILE), 0)) < 0)
+ goto out;
if (commondir_path)
git_buf_swap(&common_link, commondir_path);
break;
}
- }
- else if (S_ISREG(st.st_mode) && git__suffixcmp(path.ptr, "/" DOT_GIT) == 0) {
- error = read_gitfile(&repo_link, path.ptr);
- if (error < 0)
- break;
- if (valid_repository_path(&repo_link, &common_link)) {
+ } else if (S_ISREG(st.st_mode) && git__suffixcmp(path.ptr, "/" DOT_GIT) == 0) {
+ if ((error = read_gitfile(&repo_link, path.ptr)) < 0 ||
+ (error = is_valid_repository_path(&is_valid, &repo_link, &common_link)) < 0)
+ goto out;
+
+ if (is_valid) {
git_buf_swap(gitdir_path, &repo_link);
if (gitlink_path)
- error = git_buf_put(gitlink_path, path.ptr, path.size);
+ if ((error = git_buf_put(gitlink_path, path.ptr, path.size)) < 0)
+ goto out;
if (commondir_path)
git_buf_swap(&common_link, commondir_path);
}
@@ -523,10 +535,8 @@ static int find_repo(
/* Move up one directory. If we're in_dot_git, we'll search the
* parent itself next. If we're !in_dot_git, we'll search .git
* in the parent directory next (added at the top of the loop). */
- if (git_path_dirname_r(&path, path.ptr) < 0) {
- error = -1;
- break;
- }
+ if ((error = git_path_dirname_r(&path, path.ptr)) < 0)
+ goto out;
/* Once we've checked the directory (and .git if applicable),
* find the ceiling for a search. */
@@ -534,31 +544,28 @@ static int find_repo(
ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs);
/* Check if we should stop searching here. */
- if (min_iterations == 0
- && (path.ptr[ceiling_offset] == 0
- || (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))
+ if (min_iterations == 0 &&
+ (path.ptr[ceiling_offset] == 0 || (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))
break;
}
- if (!error && workdir_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
+ if (workdir_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
if (!git_buf_len(gitdir_path))
git_buf_clear(workdir_path);
- else {
- git_path_dirname_r(workdir_path, path.ptr);
- git_path_to_dir(workdir_path);
- }
- if (git_buf_oom(workdir_path))
- return -1;
+ else if ((error = git_path_dirname_r(workdir_path, path.ptr)) < 0 ||
+ (error = git_path_to_dir(workdir_path)) < 0)
+ goto out;
}
/* If we didn't find the repository, and we don't have any other error
* to report, report that. */
- if (!git_buf_len(gitdir_path) && !error) {
- git_error_set(GIT_ERROR_REPOSITORY,
- "could not find repository from '%s'", start_path);
+ if (!git_buf_len(gitdir_path)) {
+ git_error_set(GIT_ERROR_REPOSITORY, "could not find repository from '%s'", start_path);
error = GIT_ENOTFOUND;
+ goto out;
}
+out:
git_buf_dispose(&path);
git_buf_dispose(&repo_link);
git_buf_dispose(&common_link);
@@ -569,14 +576,16 @@ int git_repository_open_bare(
git_repository **repo_ptr,
const char *bare_path)
{
- int error;
git_buf path = GIT_BUF_INIT, common_path = GIT_BUF_INIT;
git_repository *repo = NULL;
+ bool is_valid;
+ int error;
- if ((error = git_path_prettify_dir(&path, bare_path, NULL)) < 0)
+ if ((error = git_path_prettify_dir(&path, bare_path, NULL)) < 0 ||
+ (error = is_valid_repository_path(&is_valid, &path, &common_path)) < 0)
return error;
- if (!valid_repository_path(&path, &common_path)) {
+ if (!is_valid) {
git_buf_dispose(&path);
git_buf_dispose(&common_path);
git_error_set(GIT_ERROR_REPOSITORY, "path is not a repository: %s", bare_path);
@@ -2055,6 +2064,7 @@ int git_repository_init_ext(
git_buf repo_path = GIT_BUF_INIT, wd_path = GIT_BUF_INIT,
common_path = GIT_BUF_INIT, head_path = GIT_BUF_INIT;
const char *wd;
+ bool is_valid;
int error;
assert(out && given_repo && opts);
@@ -2066,7 +2076,10 @@ int git_repository_init_ext(
wd = (opts->flags & GIT_REPOSITORY_INIT_BARE) ? NULL : git_buf_cstr(&wd_path);
- if (valid_repository_path(&repo_path, &common_path)) {
+ if ((error = is_valid_repository_path(&is_valid, &repo_path, &common_path)) < 0)
+ goto out;
+
+ if (is_valid) {
if ((opts->flags & GIT_REPOSITORY_INIT_NO_REINIT) != 0) {
git_error_set(GIT_ERROR_REPOSITORY,
"attempt to reinitialize '%s'", given_repo);