I think I've come across an out-of-bound read in libbpf.c that could
result in
potential information leak (or crash). I wanted to test it myself, but
unfortunately it will take a while for me to set up things. So, let me just
describe it and submit a candidate patch.
Codebase at: commit 6537cfb395f352782918d8ee7b7f10ba2cc3cbf2 (HEAD ->
master, origin/master, origin/HEAD)
File: ./tools/lib/bpf/libbpf.c
Function: set_kcfg_value_str
The first subscripted read of `value` (which is untrusted, IIUC) at line
2108
(`value[len - 1]`) is safe because `set_kcfg_value_str` is called only after
making sure `value` is at least one character long (although not a good
practice
in long-term). The problem is at the strip-quotes part. Here, `len -= 2` is
performed, possibly thinking `value` is at least two characters long,
since the
caller makes sure `value` has an opening quote and this function
(`set_kcfg_value_str) has a check `if (value[len - 1] != '"')` for the
closing
quote. But the problem is, both the opening and closing quotes could be the
same. More specifically, `value` could be a string that consists of a single
quote alone, and it could pass all these tests. Then `len` underflows,
resulting
in a wrap-around since it is of type `size_t`. The next branch
`if (len >= ext->kcfg.sz)` will possibly reduce the value of `len`, but will
still be greater than `strlen(value)`, making `memcpy()` at 2122 read
out-of-bound.
Suggested change:
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 194809da5172..1cc87dbd015d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2106,7 +2106,7 @@ static int set_kcfg_value_str(struct extern_desc
*ext, char *ext_val,
}
len = strlen(value);
- if (value[len - 1] != '"') {
+ if (len < 2 || value[len - 1] != '"') {
pr_warn("extern (kcfg) '%s': invalid string config '%s'\n",
ext->name, value);
return -EINVAL;
--
Nandakumar Edamana
https://nandakumar.org/
