Skip to content

Commit 5edcf5d

Browse files
committed
smart_pkt: adjust style of "ref" packet parsing function
While the function parsing ref packets doesn't have any immediately obvious buffer overflows, it's style is different to all the other parsing functions. Instead of checking buffer length while we go, it does a check up-front. This causes the code to seem a lot more magical than it really is due to some magic constants. Refactor the function to instead make use of the style of other packet parser and verify buffer lengths as we go.
1 parent 786426e commit 5edcf5d

File tree

1 file changed

+19
-25
lines changed

1 file changed

+19
-25
lines changed

src/transports/smart_pkt.c

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -206,33 +206,25 @@ static int sideband_error_pkt(git_pkt **out, const char *line, size_t len)
206206
*/
207207
static int ref_pkt(git_pkt **out, const char *line, size_t len)
208208
{
209-
int error;
210209
git_pkt_ref *pkt;
211210
size_t alloclen;
212211

213-
if (len < GIT_OID_HEXSZ + 1) {
214-
giterr_set(GITERR_NET, "error parsing pkt-line");
215-
return -1;
216-
}
217-
218-
pkt = git__malloc(sizeof(git_pkt_ref));
212+
pkt = git__calloc(1, sizeof(git_pkt_ref));
219213
GITERR_CHECK_ALLOC(pkt);
220-
221-
memset(pkt, 0x0, sizeof(git_pkt_ref));
222214
pkt->type = GIT_PKT_REF;
223-
if ((error = git_oid_fromstr(&pkt->head.oid, line)) < 0)
224-
goto error_out;
225-
226-
/* Check for a bit of consistency */
227-
if (line[GIT_OID_HEXSZ] != ' ') {
228-
giterr_set(GITERR_NET, "error parsing pkt-line");
229-
error = -1;
230-
goto error_out;
231-
}
232215

233-
/* Jump from the name */
234-
line += GIT_OID_HEXSZ + 1;
235-
len -= (GIT_OID_HEXSZ + 1);
216+
if (len < GIT_OID_HEXSZ || git_oid_fromstr(&pkt->head.oid, line) < 0)
217+
goto out_err;
218+
line += GIT_OID_HEXSZ;
219+
len -= GIT_OID_HEXSZ;
220+
221+
if (git__prefixncmp(line, len, " "))
222+
goto out_err;
223+
line++;
224+
len--;
225+
226+
if (!len)
227+
goto out_err;
236228

237229
if (line[len - 1] == '\n')
238230
--len;
@@ -244,16 +236,18 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len)
244236
memcpy(pkt->head.name, line, len);
245237
pkt->head.name[len] = '\0';
246238

247-
if (strlen(pkt->head.name) < len) {
239+
if (strlen(pkt->head.name) < len)
248240
pkt->capabilities = strchr(pkt->head.name, '\0') + 1;
249-
}
250241

251242
*out = (git_pkt *)pkt;
252243
return 0;
253244

254-
error_out:
245+
out_err:
246+
giterr_set(GITERR_NET, "error parsing REF pkt-line");
247+
if (pkt)
248+
git__free(pkt->head.name);
255249
git__free(pkt);
256-
return error;
250+
return -1;
257251
}
258252

259253
static int ok_pkt(git_pkt **out, const char *line, size_t len)

0 commit comments

Comments
 (0)