From aa36e9917ca8bc2e22fe5edd4dc08cc0f4cbe52e Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 23 Aug 2020 15:55:49 +0200 Subject: [PATCH] LibC: Prefer strlcpy over strcpy in getgrent(), fix overflow An overlong group name in /etc/groups would have caused getgrent() to overflow the global __grdb_entry. Curiously, overflow *within* __grdb_entry seems to have no detrimental effects. However, it was possible for a malicious sysadmin(?!) to craft an /etc/group that overflows outside of the page allocated for __grdb_entry thus crash the calling process. This affected at least SystemServer and su. Now, the group name will be simply truncated. For display purposes, this is fine. In case there is an exceptionally long group, it will not be properly recognized. Also, a malicious /etc/groups might cause the caller of getgrent() to become confused, but that is unavoidable. --- Libraries/LibC/grp.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Libraries/LibC/grp.cpp b/Libraries/LibC/grp.cpp index 8dc7e20c96..23eaef4cb0 100644 --- a/Libraries/LibC/grp.cpp +++ b/Libraries/LibC/grp.cpp @@ -135,12 +135,12 @@ next_entry: __grdb_entry->gr_passwd = __grdb_entry->passwd_buffer; for (size_t i = 0; i < members.size(); ++i) { __grdb_entry->members[i] = __grdb_entry->members_buffer[i]; - strcpy(__grdb_entry->members_buffer[i], members[i].characters()); + strlcpy(__grdb_entry->members_buffer[i], members[i].characters(), sizeof(__grdb_entry->members_buffer[i])); } __grdb_entry->members[members.size()] = nullptr; __grdb_entry->gr_mem = __grdb_entry->members; - strncpy(__grdb_entry->name_buffer, e_name.characters(), GRDB_STR_MAX_LEN - 1); - strncpy(__grdb_entry->passwd_buffer, e_passwd.characters(), GRDB_STR_MAX_LEN - 1); + strlcpy(__grdb_entry->name_buffer, e_name.characters(), GRDB_STR_MAX_LEN); + strlcpy(__grdb_entry->passwd_buffer, e_passwd.characters(), GRDB_STR_MAX_LEN); return __grdb_entry; }