Skip to content

Commit 1d895f1

Browse files
peffgitster
authored andcommitted
mailsplit: make PATH_MAX buffers dynamic
There are several PATH_MAX-sized buffers in mailsplit, along with some questionable uses of sprintf. These are not really of security interest, as local mailsplit pathnames are not typically under control of an attacker, and you could generally only overflow a few numbers at the end of a path that approaches PATH_MAX (a longer path would choke mailsplit long before). But it does not hurt to be careful, and as a bonus we lift some limits for systems with too-small PATH_MAX varibles. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c1fd080 commit 1d895f1

File tree

1 file changed

+23
-11
lines changed

1 file changed

+23
-11
lines changed

builtin/mailsplit.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,30 +98,37 @@ static int populate_maildir_list(struct string_list *list, const char *path)
9898
{
9999
DIR *dir;
100100
struct dirent *dent;
101-
char name[PATH_MAX];
101+
char *name = NULL;
102102
char *subs[] = { "cur", "new", NULL };
103103
char **sub;
104+
int ret = -1;
104105

105106
for (sub = subs; *sub; ++sub) {
106-
snprintf(name, sizeof(name), "%s/%s", path, *sub);
107+
free(name);
108+
name = xstrfmt("%s/%s", path, *sub);
107109
if ((dir = opendir(name)) == NULL) {
108110
if (errno == ENOENT)
109111
continue;
110112
error("cannot opendir %s (%s)", name, strerror(errno));
111-
return -1;
113+
goto out;
112114
}
113115

114116
while ((dent = readdir(dir)) != NULL) {
115117
if (dent->d_name[0] == '.')
116118
continue;
117-
snprintf(name, sizeof(name), "%s/%s", *sub, dent->d_name);
119+
free(name);
120+
name = xstrfmt("%s/%s", *sub, dent->d_name);
118121
string_list_insert(list, name);
119122
}
120123

121124
closedir(dir);
122125
}
123126

124-
return 0;
127+
ret = 0;
128+
129+
out:
130+
free(name);
131+
return ret;
125132
}
126133

127134
static int maildir_filename_cmp(const char *a, const char *b)
@@ -148,8 +155,7 @@ static int maildir_filename_cmp(const char *a, const char *b)
148155
static int split_maildir(const char *maildir, const char *dir,
149156
int nr_prec, int skip)
150157
{
151-
char file[PATH_MAX];
152-
char name[PATH_MAX];
158+
char *file = NULL;
153159
FILE *f = NULL;
154160
int ret = -1;
155161
int i;
@@ -161,7 +167,11 @@ static int split_maildir(const char *maildir, const char *dir,
161167
goto out;
162168

163169
for (i = 0; i < list.nr; i++) {
164-
snprintf(file, sizeof(file), "%s/%s", maildir, list.items[i].string);
170+
char *name;
171+
172+
free(file);
173+
file = xstrfmt("%s/%s", maildir, list.items[i].string);
174+
165175
f = fopen(file, "r");
166176
if (!f) {
167177
error("cannot open mail %s (%s)", file, strerror(errno));
@@ -173,8 +183,9 @@ static int split_maildir(const char *maildir, const char *dir,
173183
goto out;
174184
}
175185

176-
sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
186+
name = xstrfmt("%s/%0*d", dir, nr_prec, ++skip);
177187
split_one(f, name, 1);
188+
free(name);
178189

179190
fclose(f);
180191
f = NULL;
@@ -184,14 +195,14 @@ static int split_maildir(const char *maildir, const char *dir,
184195
out:
185196
if (f)
186197
fclose(f);
198+
free(file);
187199
string_list_clear(&list, 1);
188200
return ret;
189201
}
190202

191203
static int split_mbox(const char *file, const char *dir, int allow_bare,
192204
int nr_prec, int skip)
193205
{
194-
char name[PATH_MAX];
195206
int ret = -1;
196207
int peek;
197208

@@ -218,8 +229,9 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,
218229
}
219230

220231
while (!file_done) {
221-
sprintf(name, "%s/%0*d", dir, nr_prec, ++skip);
232+
char *name = xstrfmt("%s/%0*d", dir, nr_prec, ++skip);
222233
file_done = split_one(f, name, allow_bare);
234+
free(name);
223235
}
224236

225237
if (f != stdin)

0 commit comments

Comments
 (0)