Skip to content

Commit 21a2d4a

Browse files
peffgitster
authored andcommitted
transport-helper: avoid reading past end-of-string
We detect the "import-marks" capability by looking for that string, but _without_ a trailing space. Then we skip past it using strlen("import-marks "), with a space. So if a remote helper gives us exactly "import-marks", we will read past the end-of-string by one character. This is unlikely to be a problem in practice, because such input is malformed in the first place, and because there is a good chance that the string has an extra NUL terminator one character after the original (because it formerly had a newline in it that we parsed off). We can fix it by using skip_prefix with "import-marks ", with the space. The other form appears to be a typo from a515ebe (transport-helper: implement marks location as capability, 2011-07-16); "import-marks" has never existed without an argument, and it should match the "export-marks" definition above. Speaking of which, we can also use skip_prefix in a few other places while we are in the function. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ff45c0d commit 21a2d4a

File tree

1 file changed

+7
-7
lines changed

1 file changed

+7
-7
lines changed

transport-helper.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ static struct child_process *get_helper(struct transport *transport)
153153
write_constant(helper->in, "capabilities\n");
154154

155155
while (1) {
156-
const char *capname;
156+
const char *capname, *arg;
157157
int mandatory = 0;
158158
if (recvline(data, &buf))
159159
exit(128);
@@ -183,19 +183,19 @@ static struct child_process *get_helper(struct transport *transport)
183183
data->export = 1;
184184
else if (!strcmp(capname, "check-connectivity"))
185185
data->check_connectivity = 1;
186-
else if (!data->refspecs && starts_with(capname, "refspec ")) {
186+
else if (!data->refspecs && skip_prefix(capname, "refspec ", &arg)) {
187187
ALLOC_GROW(refspecs,
188188
refspec_nr + 1,
189189
refspec_alloc);
190-
refspecs[refspec_nr++] = xstrdup(capname + strlen("refspec "));
190+
refspecs[refspec_nr++] = xstrdup(arg);
191191
} else if (!strcmp(capname, "connect")) {
192192
data->connect = 1;
193193
} else if (!strcmp(capname, "signed-tags")) {
194194
data->signed_tags = 1;
195-
} else if (starts_with(capname, "export-marks ")) {
196-
data->export_marks = xstrdup(capname + strlen("export-marks "));
197-
} else if (starts_with(capname, "import-marks")) {
198-
data->import_marks = xstrdup(capname + strlen("import-marks "));
195+
} else if (skip_prefix(capname, "export-marks ", &arg)) {
196+
data->export_marks = xstrdup(arg);
197+
} else if (skip_prefix(capname, "import-marks ", &arg)) {
198+
data->import_marks = xstrdup(arg);
199199
} else if (starts_with(capname, "no-private-update")) {
200200
data->no_private_update = 1;
201201
} else if (mandatory) {

0 commit comments

Comments
 (0)