mkoegler@xxxxxxxxxxxxxxxxx (Martin Koegler) writes:
> 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?
Yes, 11031d7e9f34f6a20ff4a4bd4fa3e5e3c0024a57 seems to have been
a bit sloppy. The codepath to delete a ref may need to be lax
(see 28391a80a94d2b59d1d21f8264fe5dab91d77249) but there is no
excuse not to be strict when updating.
> 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?
Interesting. Things have been this way forever, I think. I do
not offhand see any reason not to refuse refs outside refs/, so
you can try
if (prefixcmp(name, "refs/") || check_ref_format(name + 5))
and see what happens. Some people may however want to push to
HEAD (that is ".git/HEAD" which is outside ".git/refs"), though.
> 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.
Misconversion in 8558fd9ece4c8250a037a6d5482a8040d600ef47 that
changed check_ref_format() without looking at what its callers
are checking, I think.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
