On 20/03/2024 06:28, Dan Carpenter wrote:
On Tue, Mar 19, 2024 at 03:53:24PM +0100, Neil Armstrong wrote:
While meson_sm_read_efuse() doesn't overflow, the string is not
zero terminated and env_set() will buffer overflow and add random
characters to environment.
In the Linux kernel we would give this a CVE because it's information
disclosure bug...
Yes probably
Signed-off-by: Neil Armstrong <[email protected]>
---
board/amlogic/jethub-j80/jethub-j80.c | 6 ++++--
board/amlogic/p200/p200.c | 3 ++-
board/amlogic/p201/p201.c | 3 ++-
board/amlogic/p212/p212.c | 3 ++-
board/amlogic/q200/q200.c | 3 ++-
5 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/board/amlogic/jethub-j80/jethub-j80.c
b/board/amlogic/jethub-j80/jethub-j80.c
index 185880de13..d10492cc46 100644
--- a/board/amlogic/jethub-j80/jethub-j80.c
+++ b/board/amlogic/jethub-j80/jethub-j80.c
@@ -28,8 +28,8 @@
int misc_init_r(void)
{
u8 mac_addr[EFUSE_MAC_SIZE];
^^^^^^^^^^^^^^^^^^^^^^^^
This one is also a problem. You can't pass non-terminated strings to
eth_env_set_enetaddr(). We call strlen() on it in either hsearch_r() or
env_get_from_linear().
All the other functions had a mac_addr[] issue as well.
Ack, I'll also fix those, I should have checked before...
Btw, this kind of bug is a good candidate for a static checker warning.
I can create a Smatch check for this. It would probably be easier in
Coccinelle even, but I'm the Smatch maintainer.
Would be nice!
regards,
dan carpenter
- char serial[EFUSE_SN_SIZE];
- char usid[EFUSE_USID_SIZE];
+ char serial[EFUSE_SN_SIZE + 1];
+ char usid[EFUSE_USID_SIZE + 1];
ssize_t len;
unsigned int adcval;
int ret;
@@ -46,6 +46,7 @@ int misc_init_r(void)
if (!env_get("serial")) {
len = meson_sm_read_efuse(EFUSE_SN_OFFSET, serial,
EFUSE_SN_SIZE);
+ serial[len] = '\0';
if (len == EFUSE_SN_SIZE)
env_set("serial", serial);
Thanks,
Neil
