refs.c: allow listing and deleting badly named refs
We currently do not handle badly named refs well: $ cp .git/refs/heads/master .git/refs/heads/master.....@\*@\\. $ git branch fatal: Reference has invalid format: 'refs/heads/master.....@*@\.' $ git branch -D master.....@\*@\\. error: branch 'master.....@*@\.' not found. Users cannot recover from a badly named ref without manually finding and deleting the loose ref file or appropriate line in packed-refs. Making that easier will make it easier to tweak the ref naming rules in the future, for example to forbid shell metacharacters like '`' and '"', without putting people in a state that is hard to get out of. So allow "branch --list" to show these refs and allow "branch -d/-D" and "update-ref -d" to delete them. Other commands (for example to rename refs) will continue to not handle these refs but can be changed in later patches. Details: In resolving functions, refuse to resolve refs that don't pass the git-check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME flag is passed. Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to resolve refs that escape the refs/ directory and do not match the pattern [A-Z_]* (think "HEAD" and "MERGE_HEAD"). In locking functions, refuse to act on badly named refs unless they are being deleted and either are in the refs/ directory or match [A-Z_]*. Just like other invalid refs, flag resolved, badly named refs with the REF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them in all iteration functions except for for_each_rawref. Flag badly named refs (but not symrefs pointing to badly named refs) with a REF_BAD_NAME flag to make it easier for future callers to notice and handle them specially. For example, in a later patch for-each-ref will use this flag to detect refs whose names can confuse callers parsing for-each-ref output. In the transaction API, refuse to create or update badly named refs, but allow deleting them (unless they try to escape refs/ and don't match [A-Z_]*). Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ronnie Sahlberg committed
Sep 3, 2014 at 11:45 UTC
d0f810f0bc0d6b51722b400f70c2590713f168e8
5 files changed
+273
-38
builtin/branch.c
+5
-4
@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
238
name = mkpathdup(fmt, bname.buf);
239
target = resolve_ref_unsafe(name,
240
RESOLVE_REF_READING
241
- | RESOLVE_REF_NO_RECURSE,
241
+ | RESOLVE_REF_NO_RECURSE
242
+ | RESOLVE_REF_ALLOW_BAD_NAME,
243
sha1, &flags);
244
if (!target) {
245
error(remote_branch
@@ -248,7 +249,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
249
continue;
250
}
251
251
- if (!(flags & REF_ISSYMREF) &&
252
+ if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
253
check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
254
force)) {
255
ret = 1;
@@ -268,8 +269,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
269
? _("Deleted remote branch %s (was %s).\n")
270
: _("Deleted branch %s (was %s).\n"),
271
bname.buf,
271
- (flags & REF_ISSYMREF)
272
- ? target
272
+ (flags & REF_ISBROKEN) ? "broken"
273
+ : (flags & REF_ISSYMREF) ? target
274
: find_unique_abbrev(sha1, DEFAULT_ABBREV));
275
}
276
delete_branch_config(bname.buf);
cache.h
+15
-2
@@ -981,16 +981,29 @@ extern int read_ref(const char *refname, unsigned char *sha1);
981
* If flags is non-NULL, set the value that it points to the
982
* combination of REF_ISPACKED (if the reference was found among the
983
* packed references), REF_ISSYMREF (if the initial reference was a
984
- * symbolic reference) and REF_ISBROKEN (if the ref is malformed).
984
+ * symbolic reference), REF_BAD_NAME (if the reference name is ill
985
+ * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN
986
+ * (if the ref is malformed or has a bad name). See refs.h for more detail
987
+ * on each flag.
988
*
989
* If ref is not a properly-formatted, normalized reference, return
990
* NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
991
* give up and return NULL.
992
*
990
- * errno is set to something meaningful on error.
993
+ * RESOLVE_REF_ALLOW_BAD_NAME allows resolving refs even when their
994
+ * name is invalid according to git-check-ref-format(1). If the name
995
+ * is bad then the value stored in sha1 will be null_sha1 and the two
996
+ * flags REF_ISBROKEN and REF_BAD_NAME will be set.
997
+ *
998
+ * Even with RESOLVE_REF_ALLOW_BAD_NAME, names that escape the refs/
999
+ * directory and do not consist of all caps and underscores cannot be
1000
+ * resolved. The function returns NULL for such ref names.
1001
+ * Caps and underscores refers to the special refs, such as HEAD,
1002
+ * FETCH_HEAD and friends, that all live outside of the refs/ directory.
1003
*/
1004
#define RESOLVE_REF_READING 0x01
1005
#define RESOLVE_REF_NO_RECURSE 0x02
1006
+#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
1007
extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
1008
extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
1009
refs.c
+119
-29
@@ -187,8 +187,8 @@ struct ref_dir {
187
188
/*
189
* Bit values for ref_entry::flag. REF_ISSYMREF=0x01,
190
- * REF_ISPACKED=0x02, and REF_ISBROKEN=0x04 are public values; see
191
- * refs.h.
190
+ * REF_ISPACKED=0x02, REF_ISBROKEN=0x04 and REF_BAD_NAME=0x08 are
191
+ * public values; see refs.h.
192
*/
193
194
/*
@@ -196,16 +196,16 @@ struct ref_dir {
196
* the correct peeled value for the reference, which might be
197
* null_sha1 if the reference is not a tag or if it is broken.
198
*/
199
-#define REF_KNOWS_PEELED 0x08
199
+#define REF_KNOWS_PEELED 0x10
200
201
/* ref_entry represents a directory of references */
202
-#define REF_DIR 0x10
202
+#define REF_DIR 0x20
203
204
/*
205
* Entry has not yet been read from disk (used only for REF_DIR
206
* entries representing loose references)
207
*/
208
-#define REF_INCOMPLETE 0x20
208
+#define REF_INCOMPLETE 0x40
209
210
/*
211
* A ref_entry represents either a reference or a "subdirectory" of
@@ -274,6 +274,39 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
274
return dir;
275
}
276
277
+/*
278
+ * Check if a refname is safe.
279
+ * For refs that start with "refs/" we consider it safe as long they do
280
+ * not try to resolve to outside of refs/.
281
+ *
282
+ * For all other refs we only consider them safe iff they only contain
283
+ * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
284
+ * "config").
285
+ */
286
+static int refname_is_safe(const char *refname)
287
+{
288
+ if (starts_with(refname, "refs/")) {
289
+ char *buf;
290
+ int result;
291
+
292
+ buf = xmalloc(strlen(refname) + 1);
293
+ /*
294
+ * Does the refname try to escape refs/?
295
+ * For example: refs/foo/../bar is safe but refs/foo/../../bar
296
+ * is not.
297
+ */
298
+ result = !normalize_path_copy(buf, refname + strlen("refs/"));
299
+ free(buf);
300
+ return result;
301
+ }
302
+ while (*refname) {
303
+ if (!isupper(*refname) && *refname != '_')
304
+ return 0;
305
+ refname++;
306
+ }
307
+ return 1;
308
+}
309
+
310
static struct ref_entry *create_ref_entry(const char *refname,
311
const unsigned char *sha1, int flag,
312
int check_name)
@@ -284,6 +317,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
317
if (check_name &&
318
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
319
die("Reference has invalid format: '%s'", refname);
320
+ if (!check_name && !refname_is_safe(refname))
321
+ die("Reference has invalid name: '%s'", refname);
322
len = strlen(refname) + 1;
323
ref = xmalloc(sizeof(struct ref_entry) + len);
324
hashcpy(ref->u.value.sha1, sha1);
@@ -1111,7 +1146,13 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
1146
1147
refname = parse_ref_line(refline, sha1);
1148
if (refname) {
1114
- last = create_ref_entry(refname, sha1, REF_ISPACKED, 1);
1149
+ int flag = REF_ISPACKED;
1150
+
1151
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
1152
+ hashclr(sha1);
1153
+ flag |= REF_BAD_NAME | REF_ISBROKEN;
1154
+ }
1155
+ last = create_ref_entry(refname, sha1, flag, 0);
1156
if (peeled == PEELED_FULLY ||
1157
(peeled == PEELED_TAGS && starts_with(refname, "refs/tags/")))
1158
last->flag |= REF_KNOWS_PEELED;
@@ -1249,8 +1290,13 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
1290
hashclr(sha1);
1291
flag |= REF_ISBROKEN;
1292
}
1293
+ if (check_refname_format(refname.buf,
1294
+ REFNAME_ALLOW_ONELEVEL)) {
1295
+ hashclr(sha1);
1296
+ flag |= REF_BAD_NAME | REF_ISBROKEN;
1297
+ }
1298
add_entry_to_dir(dir,
1253
- create_ref_entry(refname.buf, sha1, flag, 1));
1299
+ create_ref_entry(refname.buf, sha1, flag, 0));
1300
}
1301
strbuf_setlen(&refname, dirnamelen);
1302
}
@@ -1369,10 +1415,10 @@ static struct ref_entry *get_packed_ref(const char *refname)
1415
* A loose ref file doesn't exist; check for a packed ref. The
1416
* options are forwarded from resolve_safe_unsafe().
1417
*/
1372
-static const char *handle_missing_loose_ref(const char *refname,
1373
- int resolve_flags,
1374
- unsigned char *sha1,
1375
- int *flags)
1418
+static int resolve_missing_loose_ref(const char *refname,
1419
+ int resolve_flags,
1420
+ unsigned char *sha1,
1421
+ int *flags)
1422
{
1423
struct ref_entry *entry;
1424
@@ -1385,14 +1431,15 @@ static const char *handle_missing_loose_ref(const char *refname,
1431
hashcpy(sha1, entry->u.value.sha1);
1432
if (flags)
1433
*flags |= REF_ISPACKED;
1388
- return refname;
1434
+ return 0;
1435
}
1436
/* The reference is not a packed reference, either. */
1437
if (resolve_flags & RESOLVE_REF_READING) {
1392
- return NULL;
1438
+ errno = ENOENT;
1439
+ return -1;
1440
} else {
1441
hashclr(sha1);
1395
- return refname;
1442
+ return 0;
1443
}
1444
}
1445
@@ -1403,13 +1450,29 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1450
ssize_t len;
1451
char buffer[256];
1452
static char refname_buffer[256];
1453
+ int bad_name = 0;
1454
1455
if (flags)
1456
*flags = 0;
1457
1458
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
1411
- errno = EINVAL;
1412
- return NULL;
1459
+ if (flags)
1460
+ *flags |= REF_BAD_NAME;
1461
+
1462
+ if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
1463
+ !refname_is_safe(refname)) {
1464
+ errno = EINVAL;
1465
+ return NULL;
1466
+ }
1467
+ /*
1468
+ * dwim_ref() uses REF_ISBROKEN to distinguish between
1469
+ * missing refs and refs that were present but invalid,
1470
+ * to complain about the latter to stderr.
1471
+ *
1472
+ * We don't know whether the ref exists, so don't set
1473
+ * REF_ISBROKEN yet.
1474
+ */
1475
+ bad_name = 1;
1476
}
1477
for (;;) {
1478
char path[PATH_MAX];
@@ -1435,11 +1498,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1498
*/
1499
stat_ref:
1500
if (lstat(path, &st) < 0) {
1438
- if (errno == ENOENT)
1439
- return handle_missing_loose_ref(refname,
1440
- resolve_flags, sha1, flags);
1441
- else
1501
+ if (errno != ENOENT)
1502
+ return NULL;
1503
+ if (resolve_missing_loose_ref(refname, resolve_flags,
1504
+ sha1, flags))
1505
return NULL;
1506
+ if (bad_name) {
1507
+ hashclr(sha1);
1508
+ if (flags)
1509
+ *flags |= REF_ISBROKEN;
1510
+ }
1511
+ return refname;
1512
}
1513
1514
/* Follow "normalized" - ie "refs/.." symlinks by hand */
@@ -1512,6 +1581,11 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1581
errno = EINVAL;
1582
return NULL;
1583
}
1584
+ if (bad_name) {
1585
+ hashclr(sha1);
1586
+ if (flags)
1587
+ *flags |= REF_ISBROKEN;
1588
+ }
1589
return refname;
1590
}
1591
if (flags)
@@ -1527,8 +1601,13 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
1601
if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
1602
if (flags)
1603
*flags |= REF_ISBROKEN;
1530
- errno = EINVAL;
1531
- return NULL;
1604
+
1605
+ if (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||
1606
+ !refname_is_safe(buf)) {
1607
+ errno = EINVAL;
1608
+ return NULL;
1609
+ }
1610
+ bad_name = 1;
1611
}
1612
}
1613
}
@@ -2160,18 +2239,16 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
2239
int missing = 0;
2240
int attempts_remaining = 3;
2241
2163
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
2164
- errno = EINVAL;
2165
- return NULL;
2166
- }
2167
-
2242
lock = xcalloc(1, sizeof(struct ref_lock));
2243
lock->lock_fd = -1;
2244
2245
if (mustexist)
2246
resolve_flags |= RESOLVE_REF_READING;
2173
- if (flags & REF_NODEREF && flags & REF_DELETING)
2174
- resolve_flags |= RESOLVE_REF_NO_RECURSE;
2247
+ if (flags & REF_DELETING) {
2248
+ resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME;
2249
+ if (flags & REF_NODEREF)
2250
+ resolve_flags |= RESOLVE_REF_NO_RECURSE;
2251
+ }
2252
2253
refname = resolve_ref_unsafe(refname, resolve_flags,
2254
lock->old_sha1, &type);
@@ -3519,6 +3596,13 @@ int ref_transaction_update(struct ref_transaction *transaction,
3596
if (have_old && !old_sha1)
3597
die("BUG: have_old is true but old_sha1 is NULL");
3598
3599
+ if (!is_null_sha1(new_sha1) &&
3600
+ check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
3601
+ strbuf_addf(err, "refusing to update ref with bad name %s",
3602
+ refname);
3603
+ return -1;
3604
+ }
3605
+
3606
update = add_update(transaction, refname);
3607
hashcpy(update->new_sha1, new_sha1);
3608
update->flags = flags;
@@ -3544,6 +3628,12 @@ int ref_transaction_create(struct ref_transaction *transaction,
3628
if (!new_sha1 || is_null_sha1(new_sha1))
3629
die("BUG: create ref with null new_sha1");
3630
3631
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
3632
+ strbuf_addf(err, "refusing to create ref with bad name %s",
3633
+ refname);
3634
+ return -1;
3635
+ }
3636
+
3637
update = add_update(transaction, refname);
3638
3639
hashcpy(update->new_sha1, new_sha1);
refs.h
+10
-2
@@ -56,11 +56,19 @@ struct ref_transaction;
56
57
/*
58
* Reference cannot be resolved to an object name: dangling symbolic
59
- * reference (directly or indirectly), corrupt reference file, or
60
- * symbolic reference refers to ill-formatted reference name.
59
+ * reference (directly or indirectly), corrupt reference file,
60
+ * reference exists but name is bad, or symbolic reference refers to
61
+ * ill-formatted reference name.
62
*/
63
#define REF_ISBROKEN 0x04
64
65
+/*
66
+ * Reference name is not well formed.
67
+ *
68
+ * See git-check-ref-format(1) for the definition of well formed ref names.
69
+ */
70
+#define REF_BAD_NAME 0x08
71
+
72
/*
73
* The signature for the callback function for the for_each_*()
74
* functions below. The memory pointed to by the refname and sha1
t/t1430-bad-ref-name.sh
+124
-1
@@ -4,7 +4,8 @@ test_description='Test handling of ref names that check-ref-format rejects'
4
. ./test-lib.sh
5
6
test_expect_success setup '
7
- test_commit one
7
+ test_commit one &&
8
+ test_commit two
9
'
10
11
test_expect_success 'fast-import: fail on invalid branch name ".badbranchname"' '
@@ -37,6 +38,107 @@ test_expect_success 'fast-import: fail on invalid branch name "bad[branch]name"'
38
test_must_fail git fast-import <input
39
'
40
41
+test_expect_success 'git branch shows badly named ref' '
42
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
43
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
44
+ git branch >output &&
45
+ grep -e "broken\.\.\.ref" output
46
+'
47
+
48
+test_expect_success 'branch -d can delete badly named ref' '
49
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
50
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
51
+ git branch -d broken...ref &&
52
+ git branch >output &&
53
+ ! grep -e "broken\.\.\.ref" output
54
+'
55
+
56
+test_expect_success 'branch -D can delete badly named ref' '
57
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
58
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
59
+ git branch -D broken...ref &&
60
+ git branch >output &&
61
+ ! grep -e "broken\.\.\.ref" output
62
+'
63
+
64
+test_expect_success 'branch -D cannot delete non-ref in .git dir' '
65
+ echo precious >.git/my-private-file &&
66
+ echo precious >expect &&
67
+ test_must_fail git branch -D ../../my-private-file &&
68
+ test_cmp expect .git/my-private-file
69
+'
70
+
71
+test_expect_success 'branch -D cannot delete absolute path' '
72
+ git branch -f extra &&
73
+ test_must_fail git branch -D "$(pwd)/.git/refs/heads/extra" &&
74
+ test_cmp_rev HEAD extra
75
+'
76
+
77
+test_expect_success 'git branch cannot create a badly named ref' '
78
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
79
+ test_must_fail git branch broken...ref &&
80
+ git branch >output &&
81
+ ! grep -e "broken\.\.\.ref" output
82
+'
83
+
84
+test_expect_success 'branch -m cannot rename to a bad ref name' '
85
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
86
+ test_might_fail git branch -D goodref &&
87
+ git branch goodref &&
88
+ test_must_fail git branch -m goodref broken...ref &&
89
+ test_cmp_rev master goodref &&
90
+ git branch >output &&
91
+ ! grep -e "broken\.\.\.ref" output
92
+'
93
+
94
+test_expect_failure 'branch -m can rename from a bad ref name' '
95
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
96
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
97
+ git branch -m broken...ref renamed &&
98
+ test_cmp_rev master renamed &&
99
+ git branch >output &&
100
+ ! grep -e "broken\.\.\.ref" output
101
+'
102
+
103
+test_expect_success 'push cannot create a badly named ref' '
104
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
105
+ test_must_fail git push "file://$(pwd)" HEAD:refs/heads/broken...ref &&
106
+ git branch >output &&
107
+ ! grep -e "broken\.\.\.ref" output
108
+'
109
+
110
+test_expect_failure 'push --mirror can delete badly named ref' '
111
+ top=$(pwd) &&
112
+ git init src &&
113
+ git init dest &&
114
+
115
+ (
116
+ cd src &&
117
+ test_commit one
118
+ ) &&
119
+ (
120
+ cd dest &&
121
+ test_commit two &&
122
+ git checkout --detach &&
123
+ cp .git/refs/heads/master .git/refs/heads/broken...ref
124
+ ) &&
125
+ git -C src push --mirror "file://$top/dest" &&
126
+ git -C dest branch >output &&
127
+ ! grep -e "broken\.\.\.ref" output
128
+'
129
+
130
+test_expect_success 'rev-parse skips symref pointing to broken name' '
131
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
132
+ git branch shadow one &&
133
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
134
+ git symbolic-ref refs/tags/shadow refs/heads/broken...ref &&
135
+
136
+ git rev-parse --verify one >expect &&
137
+ git rev-parse --verify shadow >actual 2>err &&
138
+ test_cmp expect actual &&
139
+ test_i18ngrep "ignoring.*refs/tags/shadow" err
140
+'
141
+
142
test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '
143
git symbolic-ref refs/heads/badname refs/heads/broken...ref &&
144
test_when_finished "rm -f .git/refs/heads/badname" &&
@@ -45,6 +147,27 @@ test_expect_success 'update-ref --no-deref -d can delete reference to broken nam
147
test_path_is_missing .git/refs/heads/badname
148
'
149
150
+test_expect_success 'update-ref -d can delete broken name' '
151
+ cp .git/refs/heads/master .git/refs/heads/broken...ref &&
152
+ test_when_finished "rm -f .git/refs/heads/broken...ref" &&
153
+ git update-ref -d refs/heads/broken...ref &&
154
+ git branch >output &&
155
+ ! grep -e "broken\.\.\.ref" output
156
+'
157
+
158
+test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '
159
+ echo precious >.git/my-private-file &&
160
+ echo precious >expect &&
161
+ test_must_fail git update-ref -d my-private-file &&
162
+ test_cmp expect .git/my-private-file
163
+'
164
+
165
+test_expect_success 'update-ref -d cannot delete absolute path' '
166
+ git branch -f extra &&
167
+ test_must_fail git update-ref -d "$(pwd)/.git/refs/heads/extra" &&
168
+ test_cmp_rev HEAD extra
169
+'
170
+
171
test_expect_success 'update-ref --stdin fails create with bad ref name' '
172
echo "create ~a refs/heads/master" >stdin &&
173
test_must_fail git update-ref --stdin <stdin 2>err &&