Re: [PATCH v2 2/3] path: use size_t for dir_prefix length

[Junio C Hamano <gitster@pobox.com>]

K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:

> The strlen() function returns a size_t. Storing this in a standard
> signed int is a bad practice that invites overflow vulnerabilities if
> paths get absurdly long.

"a standard signed int" -> "an int variable".  There is no
"nonstandard signed int" anyway ;-)

If we were doing malloc(len) using length truncated due to integer
wraparound and then strcpy() the whole string, it would make us
write beyond the end of the allocation, but in this case, the worst
thing that can happen is that we stop comparing prematurely, which
may make us declare that buf is a path inside the directory dir when
it isn't.  The two callers of this function do not use this
miscalculated len to carry out what they do, so there is no other
damage.  It indeed would be computing a wrong result, but "overflow
vulnerabilities" is a slight exaggeration in the context of this
patch, I think.

"overflow vulnerabilities" -> "bugs due to integer wraparound".

> Switch the variable to size_t. This is safe to do because 'len' is
> strictly used as an argument to strncmp() (which expects size_t) and
> as a positive array index, involving no signed arithmetic that could
> rely on negative values.
>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
>  path.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/path.c b/path.c
> index f613d8bbd1..56be5e1726 100644
> --- a/path.c
> +++ b/path.c
> @@ -58,7 +58,7 @@ static void strbuf_cleanup_path(struct strbuf *sb)
>  
>  static int dir_prefix(const char *buf, const char *dir)
>  {
> -	int len = strlen(dir);
> +	size_t len = strlen(dir);
>  	return !strncmp(buf, dir, len) &&
>  		(is_dir_sep(buf[len]) || buf[len] == '\0');
>  }