[PATCH 24/29] ref_transaction_update(): check refname_is_safe() at a minimum
From: Michael Haggerty <hidden>
Date: 2016-06-16 02:19:02
Subsystem:
the rest · Maintainer:
Linus Torvalds
If the user has asked that a new value be set for a reference, we use check_refname_format() to verify that the reference name satisfies all of the rules. But in other cases, at least check that refname_is_safe(). Signed-off-by: Michael Haggerty <redacted> --- There are remaining problems in this area of the code. For example, check_refname_format() is *less* strict than refname_is_safe(). It allows almost arbitrary top-level reference names like "foo" and "refs". (The latter is especially fun because if you manage to create a reference called "refs", Git stops recognizing the directory as a Git repository.) On the other hand, some callers call check_refname_format() with incomplete reference names (e.g., branch names like "master"), so the functions can't be made stricter until those callers are changed. I'll address these problems separately if I find the time. refs.c | 5 +++-- t/t1400-update-ref.sh | 2 +- t/t1430-bad-ref-name.sh | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index 858b6d7..41eb9e2 100644
--- a/refs.c
+++ b/refs.c@@ -802,8 +802,9 @@ int ref_transaction_update(struct ref_transaction *transaction, if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF)) die("BUG: REF_ISPRUNING set without REF_NODEREF"); - if (new_sha1 && !is_null_sha1(new_sha1) && - check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) { + if ((new_sha1 && !is_null_sha1(new_sha1)) ? + check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) : + !refname_is_safe(refname)) { strbuf_addf(err, "refusing to update ref with bad name '%s'", refname); return -1;
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 40b0cce..08bd8fd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh@@ -23,7 +23,7 @@ test_expect_success setup ' m=refs/heads/master n_dir=refs/heads/gu n=$n_dir/fixes -outside=foo +outside=refs/foo test_expect_success \ "create $m" \
diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh
index 25ddab4..8937e25 100755
--- a/t/t1430-bad-ref-name.sh
+++ b/t/t1430-bad-ref-name.sh@@ -285,7 +285,7 @@ test_expect_success 'update-ref -d cannot delete non-ref in .git dir' ' echo precious >expect && test_must_fail git update-ref -d my-private-file >output 2>error && test_must_be_empty output && - test_i18ngrep -e "cannot lock .*: unable to resolve reference" error && + test_i18ngrep -e "refusing to update ref with bad name" error && test_cmp expect .git/my-private-file '
--
2.8.1