Skip to content

Commit d0f810f

Browse files
rsahlberggitster
authored andcommitted
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 <[email protected]> Signed-off-by: Jonathan Nieder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8159f4a commit d0f810f

File tree

5 files changed

+273
-38
lines changed

5 files changed

+273
-38
lines changed

builtin/branch.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
238238
name = mkpathdup(fmt, bname.buf);
239239
target = resolve_ref_unsafe(name,
240240
RESOLVE_REF_READING
241-
| RESOLVE_REF_NO_RECURSE,
241+
| RESOLVE_REF_NO_RECURSE
242+
| RESOLVE_REF_ALLOW_BAD_NAME,
242243
sha1, &flags);
243244
if (!target) {
244245
error(remote_branch
@@ -248,7 +249,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
248249
continue;
249250
}
250251

251-
if (!(flags & REF_ISSYMREF) &&
252+
if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
252253
check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
253254
force)) {
254255
ret = 1;
@@ -268,8 +269,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
268269
? _("Deleted remote branch %s (was %s).\n")
269270
: _("Deleted branch %s (was %s).\n"),
270271
bname.buf,
271-
(flags & REF_ISSYMREF)
272-
? target
272+
(flags & REF_ISBROKEN) ? "broken"
273+
: (flags & REF_ISSYMREF) ? target
273274
: find_unique_abbrev(sha1, DEFAULT_ABBREV));
274275
}
275276
delete_branch_config(bname.buf);

cache.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -981,16 +981,29 @@ extern int read_ref(const char *refname, unsigned char *sha1);
981981
* If flags is non-NULL, set the value that it points to the
982982
* combination of REF_ISPACKED (if the reference was found among the
983983
* 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.
985988
*
986989
* If ref is not a properly-formatted, normalized reference, return
987990
* NULL. If more than MAXDEPTH recursive symbolic lookups are needed,
988991
* give up and return NULL.
989992
*
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.
9911003
*/
9921004
#define RESOLVE_REF_READING 0x01
9931005
#define RESOLVE_REF_NO_RECURSE 0x02
1006+
#define RESOLVE_REF_ALLOW_BAD_NAME 0x04
9941007
extern const char *resolve_ref_unsafe(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
9951008
extern char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags);
9961009

refs.c

Lines changed: 119 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -187,25 +187,25 @@ struct ref_dir {
187187

188188
/*
189189
* 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.
192192
*/
193193

194194
/*
195195
* The field ref_entry->u.value.peeled of this value entry contains
196196
* the correct peeled value for the reference, which might be
197197
* null_sha1 if the reference is not a tag or if it is broken.
198198
*/
199-
#define REF_KNOWS_PEELED 0x08
199+
#define REF_KNOWS_PEELED 0x10
200200

201201
/* ref_entry represents a directory of references */
202-
#define REF_DIR 0x10
202+
#define REF_DIR 0x20
203203

204204
/*
205205
* Entry has not yet been read from disk (used only for REF_DIR
206206
* entries representing loose references)
207207
*/
208-
#define REF_INCOMPLETE 0x20
208+
#define REF_INCOMPLETE 0x40
209209

210210
/*
211211
* 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)
274274
return dir;
275275
}
276276

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+
277310
static struct ref_entry *create_ref_entry(const char *refname,
278311
const unsigned char *sha1, int flag,
279312
int check_name)
@@ -284,6 +317,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
284317
if (check_name &&
285318
check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
286319
die("Reference has invalid format: '%s'", refname);
320+
if (!check_name && !refname_is_safe(refname))
321+
die("Reference has invalid name: '%s'", refname);
287322
len = strlen(refname) + 1;
288323
ref = xmalloc(sizeof(struct ref_entry) + len);
289324
hashcpy(ref->u.value.sha1, sha1);
@@ -1111,7 +1146,13 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
11111146

11121147
refname = parse_ref_line(refline, sha1);
11131148
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);
11151156
if (peeled == PEELED_FULLY ||
11161157
(peeled == PEELED_TAGS && starts_with(refname, "refs/tags/")))
11171158
last->flag |= REF_KNOWS_PEELED;
@@ -1249,8 +1290,13 @@ static void read_loose_refs(const char *dirname, struct ref_dir *dir)
12491290
hashclr(sha1);
12501291
flag |= REF_ISBROKEN;
12511292
}
1293+
if (check_refname_format(refname.buf,
1294+
REFNAME_ALLOW_ONELEVEL)) {
1295+
hashclr(sha1);
1296+
flag |= REF_BAD_NAME | REF_ISBROKEN;
1297+
}
12521298
add_entry_to_dir(dir,
1253-
create_ref_entry(refname.buf, sha1, flag, 1));
1299+
create_ref_entry(refname.buf, sha1, flag, 0));
12541300
}
12551301
strbuf_setlen(&refname, dirnamelen);
12561302
}
@@ -1369,10 +1415,10 @@ static struct ref_entry *get_packed_ref(const char *refname)
13691415
* A loose ref file doesn't exist; check for a packed ref. The
13701416
* options are forwarded from resolve_safe_unsafe().
13711417
*/
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)
13761422
{
13771423
struct ref_entry *entry;
13781424

@@ -1385,14 +1431,15 @@ static const char *handle_missing_loose_ref(const char *refname,
13851431
hashcpy(sha1, entry->u.value.sha1);
13861432
if (flags)
13871433
*flags |= REF_ISPACKED;
1388-
return refname;
1434+
return 0;
13891435
}
13901436
/* The reference is not a packed reference, either. */
13911437
if (resolve_flags & RESOLVE_REF_READING) {
1392-
return NULL;
1438+
errno = ENOENT;
1439+
return -1;
13931440
} else {
13941441
hashclr(sha1);
1395-
return refname;
1442+
return 0;
13961443
}
13971444
}
13981445

@@ -1403,13 +1450,29 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
14031450
ssize_t len;
14041451
char buffer[256];
14051452
static char refname_buffer[256];
1453+
int bad_name = 0;
14061454

14071455
if (flags)
14081456
*flags = 0;
14091457

14101458
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;
14131476
}
14141477
for (;;) {
14151478
char path[PATH_MAX];
@@ -1435,11 +1498,17 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
14351498
*/
14361499
stat_ref:
14371500
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))
14421505
return NULL;
1506+
if (bad_name) {
1507+
hashclr(sha1);
1508+
if (flags)
1509+
*flags |= REF_ISBROKEN;
1510+
}
1511+
return refname;
14431512
}
14441513

14451514
/* Follow "normalized" - ie "refs/.." symlinks by hand */
@@ -1512,6 +1581,11 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
15121581
errno = EINVAL;
15131582
return NULL;
15141583
}
1584+
if (bad_name) {
1585+
hashclr(sha1);
1586+
if (flags)
1587+
*flags |= REF_ISBROKEN;
1588+
}
15151589
return refname;
15161590
}
15171591
if (flags)
@@ -1527,8 +1601,13 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
15271601
if (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {
15281602
if (flags)
15291603
*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;
15321611
}
15331612
}
15341613
}
@@ -2160,18 +2239,16 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
21602239
int missing = 0;
21612240
int attempts_remaining = 3;
21622241

2163-
if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
2164-
errno = EINVAL;
2165-
return NULL;
2166-
}
2167-
21682242
lock = xcalloc(1, sizeof(struct ref_lock));
21692243
lock->lock_fd = -1;
21702244

21712245
if (mustexist)
21722246
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+
}
21752252

21762253
refname = resolve_ref_unsafe(refname, resolve_flags,
21772254
lock->old_sha1, &type);
@@ -3519,6 +3596,13 @@ int ref_transaction_update(struct ref_transaction *transaction,
35193596
if (have_old && !old_sha1)
35203597
die("BUG: have_old is true but old_sha1 is NULL");
35213598

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+
35223606
update = add_update(transaction, refname);
35233607
hashcpy(update->new_sha1, new_sha1);
35243608
update->flags = flags;
@@ -3544,6 +3628,12 @@ int ref_transaction_create(struct ref_transaction *transaction,
35443628
if (!new_sha1 || is_null_sha1(new_sha1))
35453629
die("BUG: create ref with null new_sha1");
35463630

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+
35473637
update = add_update(transaction, refname);
35483638

35493639
hashcpy(update->new_sha1, new_sha1);

refs.h

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,19 @@ struct ref_transaction;
5656

5757
/*
5858
* 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.
6162
*/
6263
#define REF_ISBROKEN 0x04
6364

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+
6472
/*
6573
* The signature for the callback function for the for_each_*()
6674
* functions below. The memory pointed to by the refname and sha1

0 commit comments

Comments
 (0)