Browse files

lockfile: fix buffer overflow in path handling

The path of the file to be locked is held in lock_file::filename,
which is a fixed-length buffer of length PATH_MAX.  This buffer is
also (temporarily) used to hold the path of the lock file, which is
the path of the file being locked plus ".lock".  Because of this, the
path of the file being locked must be less than (PATH_MAX - 5)
characters long (5 chars are needed for ".lock" and one character for
the NUL terminator).

On entry into lock_file(), the path length was only verified to be
less than PATH_MAX characters, not less than (PATH_MAX - 5)

When and if resolve_symlink() is called, then that function is
correctly told to treat the buffer as (PATH_MAX - 5) characters long.
This part is correct.  However:

* If LOCK_NODEREF was specified, then resolve_symlink() is never

* If resolve_symlink() is called but the path is not a symlink, then
  the length check is never applied.

So it is possible for a path with length (PATH_MAX - 5 <= len <
PATH_MAX) to make it through the checks.  When ".lock" is strcat()ted
to such a path, the lock_file::filename buffer is overflowed.

Fix the problem by adding a check when entering lock_file() that the
original path is less than (PATH_MAX - 5) characters.

[jc: with independent development by Peff]

Signed-off-by: Michael Haggerty <>
Signed-off-by: Junio C Hamano <>
  • Loading branch information...
1 parent 1599999 commit 2fbd4f92fa0d6d59d01cf1b9c800d428cd95143d @mhagger mhagger committed with gitster Jul 6, 2013
Showing with 6 additions and 4 deletions.
  1. +6 −4 lockfile.c
@@ -124,15 +124,17 @@ static char *resolve_symlink(char *p, size_t s)
static int lock_file(struct lock_file *lk, const char *path, int flags)
- if (strlen(path) >= sizeof(lk->filename))
- return -1;
- strcpy(lk->filename, path);
* subtract 5 from size to make sure there's room for adding
* ".lock" for the lock file name
+ static const size_t max_path_len = sizeof(lk->filename) - 5;
+ if (strlen(path) >= max_path_len)
+ return -1;
+ strcpy(lk->filename, path);
if (!(flags & LOCK_NODEREF))
- resolve_symlink(lk->filename, sizeof(lk->filename)-5);
+ resolve_symlink(lk->filename, max_path_len);
strcat(lk->filename, ".lock");
lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 <= lk->fd) {

0 comments on commit 2fbd4f9

Please sign in to comment.