Re: [PATCH v3/GSoC 2/5] path.c: implement xdg_runtime_dir()

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 25, 2016, 16:55 UTC
Message-ID: <xmqqpoui4huo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAKqreux8FHdJoKDishjQkbi9g1oUc265EUK4nOJ_sgeFivGSNA@mail.gmail.com>

惠轶群 <huiyiqun@gmail.com> writes:

Show 17 quoted lines

>> There's a lot of "what" here that the caller doesn't really care about,
>> and which may go stale with respect to the implementation over time. Can
>> we make something more succinct like:
>>
>>   /*
>>    * Return a path suitable for writing run-time files related to git,
>>    * or NULL if no such path can be established. The resulting string
>>    * should be freed by the caller.
>>    */
>>
>> ?
>
> That's clearer, but if I were the caller, I would worry about the
> security of the path.
> How about adding:
>
> The security of the path is ensured by file permission.

Is "by file permission" descriptive enough?

To protect /a/b/c/socket, what filesystem entities have the right permission bits set? If the parent directory is writable by an attacker, the permission bits on 'socket' itself may not matter as the attacker can rename it away and create new one herself, for example.

Show 6 quoted lines

> I will deal with it.
>
> I find there are some similar leakage in this file. I'll fix them in
> another patch.
>
> Do you think we need some additional comments for the release of strbuf?

As Documentation/technical/api-strbuf.txt has this, I think we are already OK.

`strbuf_release`::

	Release a string buffer and the memory it used. You should not use the
	string buffer after using this function, unless you initialize it again.