Skip to content

Commit

Permalink
attr: fix overflow when upserting attribute with overly long name
Browse files Browse the repository at this point in the history
The function `git_attr_internal()` is called to upsert attributes into
the global map. And while all callers pass a `size_t`, the function
itself accepts an `int` as the attribute name's length. This can lead to
an integer overflow in case the attribute name is longer than `INT_MAX`.

Now this overflow seems harmless as the first thing we do is to call
`attr_name_valid()`, and that function only succeeds in case all chars
in the range of `namelen` match a certain small set of chars. We thus
can't do an out-of-bounds read as NUL is not part of that set and all
strings passed to this function are NUL-terminated. And furthermore, we
wouldn't ever read past the current attribute name anyway due to the
same reason. And if validation fails we will return early.

On the other hand it feels fragile to rely on this behaviour, even more
so given that we pass `namelen` to `FLEX_ALLOC_MEM()`. So let's instead
just do the correct thing here and accept a `size_t` as line length.

Signed-off-by: Patrick Steinhardt <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
  • Loading branch information
pks-t authored and gitster committed Dec 5, 2022
1 parent abd4d67 commit eb22e7d
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ static void report_invalid_attr(const char *name, size_t len,
* dictionary. If no entry is found, create a new attribute and store it in
* the dictionary.
*/
static const struct git_attr *git_attr_internal(const char *name, int namelen)
static const struct git_attr *git_attr_internal(const char *name, size_t namelen)
{
struct git_attr *a;

Expand Down

0 comments on commit eb22e7d

Please sign in to comment.