stagit

check path truncation

be strict about it

Hiltjo Posthuma contact@arjunchoudhary.com

commit: b6d461d parent: eed5f0f
3 files changed, 56 insertions(+), 25 deletions(-)
MTODO+0-2
Mstagit-index.c+18-6
Mstagit.c+38-17
M · TODO +0, -2
1@@ -1,5 +1,3 @@
2-check path truncation? snprintf(), strlcpy.
3-
4 performance:
5 - optimize git_diff_get_stats.
6 - speed up generating files.
M · stagit-index.c +18, -6
 1@@ -178,7 +178,7 @@ main(int argc, char *argv[])
 2 	const git_error *e = NULL;
 3 	FILE *fp;
 4 	char path[PATH_MAX], *p;
 5-	int i, ret = 0;
 6+	int i, r, ret = 0;
 7 
 8 	if (argc < 2) {
 9 		fprintf(stderr, "%s [repodir...]\n", argv[0]);
10@@ -199,18 +199,24 @@ main(int argc, char *argv[])
11 			continue;
12 		}
13 
14-		/* use directory name as name */
15+		/* use directory name as name, truncation of name is no problem. */
16 		p = xbasename(repodir);
17 		snprintf(name, sizeof(name), "%s", p);
18 		free(p);
19 
20 		/* read description or .git/description */
21 		description[0] = '\0';
22-		snprintf(path, sizeof(path), "%s%s%s",
23+		r = snprintf(path, sizeof(path), "%s%s%s",
24 			repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "description");
25+		if (r == -1 || (size_t)r >= sizeof(path))
26+			errx(1, "path truncated: '%s%s%s'",
27+			        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "description");
28 		if (!(fp = fopen(path, "r"))) {
29-			snprintf(path, sizeof(path), "%s%s%s",
30+			r = snprintf(path, sizeof(path), "%s%s%s",
31 				repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/description");
32+			if (r == -1 || (size_t)r >= sizeof(path))
33+				errx(1, "path truncated: '%s%s%s'",
34+				        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/description");
35 			fp = fopen(path, "r");
36 		}
37 		if (fp) {
38@@ -221,11 +227,17 @@ main(int argc, char *argv[])
39 
40 		/* read owner or .git/owner */
41 		owner[0] = '\0';
42-		snprintf(path, sizeof(path), "%s%s%s",
43+		r = snprintf(path, sizeof(path), "%s%s%s",
44 			repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "owner");
45+		if (r == -1 || (size_t)r >= sizeof(path))
46+			errx(1, "path truncated: '%s%s%s'",
47+			        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "owner");
48 		if (!(fp = fopen(path, "r"))) {
49-			snprintf(path, sizeof(path), "%s%s%s",
50+			r = snprintf(path, sizeof(path), "%s%s%s",
51 				repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/owner");
52+			if (r == -1 || (size_t)r >= sizeof(path))
53+				errx(1, "path truncated: '%s%s%s'",
54+				        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/owner");
55 			fp = fopen(path, "r");
56 		}
57 		if (fp) {
M · stagit.c +38, -17
  1@@ -188,7 +188,8 @@ mkdirp(const char *path)
  2 {
  3 	char tmp[PATH_MAX], *p;
  4 
  5-	strlcpy(tmp, path, sizeof(tmp));
  6+	if (strlcpy(tmp, path, sizeof(tmp)) >= sizeof(tmp))
  7+		errx(1, "path truncated: '%s'", path);
  8 	for (p = tmp + (tmp[0] == '/'); *p; p++) {
  9 		if (*p != '/')
 10 			continue;
 11@@ -426,6 +427,7 @@ writelog(FILE *fp, const git_oid *oid)
 12 	size_t len;
 13 	char path[PATH_MAX];
 14 	FILE *fpfile;
 15+	int r;
 16 
 17 	git_revwalk_new(&w, repo);
 18 	git_revwalk_push(w, oid);
 19@@ -469,7 +471,10 @@ writelog(FILE *fp, const git_oid *oid)
 20 
 21 		relpath = "../";
 22 
 23-		snprintf(path, sizeof(path), "commit/%s.html", ci->oid);
 24+		r = snprintf(path, sizeof(path), "commit/%s.html", ci->oid);
 25+		if (r == -1 || (size_t)r >= sizeof(path))
 26+			errx(1, "path truncated: 'commit/%s.html'", ci->oid);
 27+
 28 		/* check if file exists if so skip it */
 29 		if (access(path, F_OK)) {
 30 			fpfile = efopen(path, "w");
 31@@ -591,8 +596,8 @@ writeblob(git_object *obj, const char *fpath, const char *filename, git_off_t fi
 32 
 33 	p = fpath;
 34 	while (*p) {
 35-		if (*p == '/')
 36-			strlcat(tmp, "../", sizeof(tmp));
 37+		if (*p == '/' && strlcat(tmp, "../", sizeof(tmp)) >= sizeof(tmp))
 38+			errx(1, "path truncated: '../%s'", tmp);
 39 		p++;
 40 	}
 41 	relpath = tmp;
 42@@ -670,7 +675,7 @@ writefilestree(FILE *fp, git_tree *tree, const char *branch, const char *path)
 43 	git_object *obj = NULL;
 44 	git_off_t filesize;
 45 	size_t count, i;
 46-	int lc, ret;
 47+	int lc, r, ret;
 48 
 49 	count = git_tree_entrycount(tree);
 50 	for (i = 0; i < count; i++) {
 51@@ -678,8 +683,11 @@ writefilestree(FILE *fp, git_tree *tree, const char *branch, const char *path)
 52 		    git_tree_entry_to_object(&obj, repo, entry))
 53 			return -1;
 54 		entryname = git_tree_entry_name(entry);
 55-		snprintf(entrypath, sizeof(entrypath), "%s%s%s",
 56+		r = snprintf(entrypath, sizeof(entrypath), "%s%s%s",
 57 			 path, path[0] ? "/" : "", entryname);
 58+		if (r == -1 || (size_t)r >= sizeof(entrypath))
 59+			errx(1, "path truncated: '%s%s%s'",
 60+			        path, path[0] ? "/" : "", entryname);
 61 		switch (git_object_type(obj)) {
 62 		case GIT_OBJ_BLOB:
 63 			break;
 64@@ -695,12 +703,13 @@ writefilestree(FILE *fp, git_tree *tree, const char *branch, const char *path)
 65 			git_object_free(obj);
 66 			continue;
 67 		}
 68-		if (path[0])
 69-			snprintf(filepath, sizeof(filepath), "file/%s/%s.html",
 70-			         path, entryname);
 71-		else
 72-			snprintf(filepath, sizeof(filepath), "file/%s.html",
 73-			         entryname);
 74+
 75+		r = snprintf(filepath, sizeof(filepath), "file/%s%s%s.html",
 76+		         path, path[0] ? "/" : "", entryname);
 77+		if (r == -1 || (size_t)r >= sizeof(filepath))
 78+			errx(1, "path truncated: 'file/%s%s%s.html'",
 79+			        path, path[0] ? "/" : "", entryname);
 80+
 81 		filesize = git_blob_rawsize((git_blob *)obj);
 82 
 83 		lc = writeblob(obj, filepath, entryname, filesize);
 84@@ -868,7 +877,7 @@ main(int argc, char *argv[])
 85 	const git_error *e = NULL;
 86 	FILE *fp, *fpread;
 87 	char path[PATH_MAX], *p;
 88-	int status;
 89+	int r, status;
 90 
 91 	if (argc != 2) {
 92 		fprintf(stderr, "%s <repodir>\n", argv[0]);
 93@@ -902,11 +911,17 @@ main(int argc, char *argv[])
 94 			*p = '\0';
 95 
 96 	/* read description or .git/description */
 97-	snprintf(path, sizeof(path), "%s%s%s",
 98+	r = snprintf(path, sizeof(path), "%s%s%s",
 99 		repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "description");
100+	if (r == -1 || (size_t)r >= sizeof(path))
101+		errx(1, "path truncated: '%s%s%s'",
102+	                repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "description");
103 	if (!(fpread = fopen(path, "r"))) {
104-		snprintf(path, sizeof(path), "%s%s%s",
105+		r = snprintf(path, sizeof(path), "%s%s%s",
106 			repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/description");
107+		if (r == -1 || (size_t)r >= sizeof(path))
108+			errx(1, "path truncated: '%s%s%s'",
109+		                repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/description");
110 		fpread = fopen(path, "r");
111 	}
112 	if (fpread) {
113@@ -916,11 +931,17 @@ main(int argc, char *argv[])
114 	}
115 
116 	/* read url or .git/url */
117-	snprintf(path, sizeof(path), "%s%s%s",
118+	r = snprintf(path, sizeof(path), "%s%s%s",
119 		repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "url");
120+	if (r == -1 || (size_t)r >= sizeof(path))
121+		errx(1, "path truncated: '%s%s%s'",
122+		        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", "url");
123 	if (!(fpread = fopen(path, "r"))) {
124-		snprintf(path, sizeof(path), "%s%s%s",
125+		r = snprintf(path, sizeof(path), "%s%s%s",
126 			repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/url");
127+		if (r == -1 || (size_t)r >= sizeof(path))
128+			errx(1, "path truncated: '%s%s%s'",
129+			        repodir, repodir[strlen(repodir)] == '/' ? "" : "/", ".git/url");
130 		fpread = fopen(path, "r");
131 	}
132 	if (fpread) {