git/list[1] front-page[2] threads[3] people[4] search[5] about
 

input validation in receive-pack

From
MKMartin Koegler <mkoegler@auto.tuwien.ac.at>
Date
Jan 1, 2008, 21:34 UTC
Message-ID
<20080101213451.GA26772@auto.tuwien.ac.at>

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
Next: Junio C Hamano
Message 1 of 8 in “input validation in receive-pack”
  1. Martin KoeglerJan 1, 2008
  2. Junio C HamanoJan 2, 2008
  3. Daniel BarkalowJan 2, 2008
  4. Junio C HamanoJan 2, 2008
  5. Daniel BarkalowJan 2, 2008
  6. Junio C HamanoJan 2, 2008
  7. Martin KoeglerJan 2, 2008
  8. Junio C HamanoJan 2, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.