]> git.ipfire.org Git - thirdparty/git.git/commit - refs.c
Fix broken sha1 locking
authorPetr Baudis <pasky@suse.cz>
Tue, 19 Sep 2006 20:58:23 +0000 (22:58 +0200)
committerJunio C Hamano <junkio@cox.net>
Wed, 20 Sep 2006 14:56:03 +0000 (07:56 -0700)
commit53cce84c05d3cba237ab45540b1e882be9c97215
tree82b3376b295c4a2e7e22b8b3cad82a84b6b8313c
parent5b10b091139ea5ef87376998c89f3a20dd7c7793
Fix broken sha1 locking

Current git#next is totally broken wrt. cloning over HTTP, generating refs
at random directories. Of course it's caused by the static get_pathname()
buffer. lock_ref_sha1() stores return value of mkpath()'s get_pathname()
call, then calls lock_ref_sha1_basic() which calls git_path(ref) which
calls get_pathname() at that point returning pointer to the same buffer.
So now you are sprintf()ing a format string into itself, wow! The resulting
pathnames are really cute. (If you've been paying attention, yes, the
mere fact that a format string _could_ write over itself is very wrong
and probably exploitable here. See the other mail I've just sent.)

I've never liked how we use return values of those functions so liberally,
the "allow some random number of get_pathname() return values to work
concurrently" is absolutely horrible pit and we've already fallen in this
before IIRC. I consider it an awful coding practice, you add a call
somewhere and at some other point some distant caller of that breaks since
it reuses the same return values. Not to mention this takes quite some time
to debug.

My gut feeling tells me that there might be more of this.  I don't have
time to review the rest of the users of the refs.c functions though.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Junio C Hamano <junkio@cox.net>
refs.c