As an actual example, suppose there is an entry as below in a device table that makedevs receives:


/dev/system        c       0660    root       system      1000      0      -       -       -


then you will get "system" group for id_buf in convert2guid().

and suppose there are two entries in /etc/group where both start with "system" like:


systemd-journal:x:900:

system:x:2020:


Obviously convert2guid() should return 2020 in this case, but with the existing code it will return 900 because the entry of "systemd-journal" comes first in the loop and strncmp("systemd-journal", "system", strlen("system")) returns 0.



Best regards,

---
Jaeyoon Jung

---------- Original Message ----------

From : "Richard Purdie" <[email protected]>
To : 정재윤 연구위원/Task Leader(jaeyoon.jung)
Cc : [email protected]
Date : 2024/08/26 20:06:58 [GMT+09:00]
Subject : Re: [OE-core] [PATCH] makedevs: Fix matching uid/gid


On Mon, 2024-08-26 at 19:07 +0900, Jung Jaeyoon(Jay) wrote:
> On Sat, Aug 17, 2024 at 06:47 AM, Richard Purdie wrote:
> > I'm afraid I don't understand the problem here. If partial matching
> > is occurring, doesn't that mean MAX_ID_LEN is wrong?
> > 
> > If we drop the strncmp() here, that appears to put us at risk of
> > buffer overflow problems, particularly if MAX_ID_LEN is too short?
> > 
> > Can you give an example of how this is failing and where the above
> > change helps?
> 
> 
> (Sorry for being late responding on this. It has been missing in my
> INBOX somehow.)
> 
> A typical case where it is failing is when id_buf is shorter than
> node->name and they match partially, e.g, id_buf is "foo" and node-
> >name is "foobar".
> Note that it's using strlen(id_buf), not strlen(MAX_ID_LEN).

convert2guid, where the strlen is located, is called from two places
both in interpret_table_entry() and there the buffer is defined as:

	char usr_buf[MAX_ID_LEN];
	char grp_buf[MAX_ID_LEN];

The name field of node is defined as:

char name[MAX_NAME_LEN+1]

which could be a problem if they differ but:

#define MAX_ID_LEN      40
#define MAX_NAME_LEN    40

so the only issue I can see is there is perhaps an off by one on the
length definition?

> I believe using strcmp() here is safe because it is guaranteed that
> both strings cannot be longer than MAX_ID_LEN(=MAX_NAME_LEN) and are
> always NULL-terminated.

Perhaps, but I think the existing code should work and if there is a
bug, I'd like to understand what it is. Is the issue an off by one on
the length?

I'd ask again if you have an actual example of the failure? Is it only
with strings of 39/40 length?

Cheers,

Richard



-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#203751): 
https://lists.openembedded.org/g/openembedded-core/message/203751
Mute This Topic: https://lists.openembedded.org/mt/107888859/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to