input validation in receive-pack
From: Martin Koegler <hidden>
Date: 2016-06-15 22:44:02
In the function "static const char *update(struct command *cmd)" in
receive-pack.c:
| if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
| !is_null_sha1(old_sha1) &&
| !prefixcmp(name, "refs/heads/")) {
| struct commit *old_commit, *new_commit;
| struct commit_list *bases, *ent;
|
| old_commit = (struct commit *)parse_object(old_sha1);
| new_commit = (struct commit *)parse_object(new_sha1);
| bases = get_merge_bases(old_commit, new_commit, 1);
| for (ent = bases; ent; ent = ent->next)
| if (!hashcmp(old_sha1, ent->item->object.sha1))
| break;
| free_commit_list(bases);
| if (!ent) {
| error("denying non-fast forward %s"
| " (you should pull first)", name);
| return "non-fast forward";
| }
| }
As far as I understand the code, it assumes, that sha1 values provided
by the client really point to a commit. Shouldn't there be a check for
the object type?
Some lines above:
| if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
| error("refusing to create funny ref '%s' remotely", name);
| return "funny refname";
| }
Is this code really correct? All refnames starting with "refs/" may
only contain allowed characters, while all other may contain any
characters (except \0 and \n)?
For the updating code path, lock_any_ref_for_update calls
check_ref_format, so the error will happen latter. For the delete code
path, the refname seems not to be checked.
In the update code path, the check is done in refs.c:
| struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *old_sha1, int flags)
| {
| if (check_ref_format(ref) == -1)
| return NULL;
| return lock_ref_sha1_basic(ref, old_sha1, flags, NULL);
| }
check_ref_format may also return -2 (less than two name levels) and -3
(* at the end), which are ignored. Is it really intended, that
receive-pack can create such refs.
mfg Martin Kögler