shadow-maint / shadow

Upstream shadow tree
Other
299 stars 230 forks source link

src/groupmod.c: bug; possibly use-after-free #1013

Closed alejandro-colomar closed 4 months ago

alejandro-colomar commented 4 months ago

Originally reproduced here:

Reported-by: @gOo0se

That PR reports a crash that is reproducible with:

[root@localhost src]# ./useradd u1
[root@localhost src]# ./useradd u2
[root@localhost src]# ./useradd u3
[root@localhost src]# ./useradd u4
[root@localhost src]# ./groupadd -U u1,u2,u3,u4 g1
[root@localhost src]# ./groupmod -n g2 -U u1,u2 g1
Segmentation fault

@gOo0se reproduced it on the master branch, and I've reproduced it on Debian Sid (shadow 4.13).

That PR originally diagnosed a double-free, which seems incorrect according to my investigation. It is more likely a use-after-free.

The crash can be traced to this path:

https://github.com/shadow-maint/shadow/blob/69f74dbf8a2341aa73dc3f2213102eae1a3e49e0/src/groupmod.c#L857 https://github.com/shadow-maint/shadow/blob/69f74dbf8a2341aa73dc3f2213102eae1a3e49e0/src/groupmod.c#L470 https://github.com/shadow-maint/shadow/blob/69f74dbf8a2341aa73dc3f2213102eae1a3e49e0/lib/commonio.c#L981 https://github.com/shadow-maint/shadow/blob/69f74dbf8a2341aa73dc3f2213102eae1a3e49e0/lib/commonio.c#L879 https://github.com/shadow-maint/shadow/blob/69f74dbf8a2341aa73dc3f2213102eae1a3e49e0/lib/groupio.c#L64

It doesn't enter the valid_field() call, which makes me suspect that gr is a freed pointer. Since it's reading *gr free(3)d memory for getting gr->gr_name, that's an easy crash.

alejandro-colomar commented 4 months ago

Still, I'm not certain of the reason of the crash; only of the location.

I can't find the place where the pointer could have been free(3)d before that, though.

gOo0se commented 4 months ago

Obviously in write_all it is a use-after-free (actually I never think it is not a use-after-free), but in some cases it is double-free, it is easier to locate double-free so I not mentioned use-after-free in #1007

The following is a screenshot of the test at that time.

core-dump double-free

gOo0se commented 4 months ago

Obviously in write_all it is a use-after-free (actually I never think it is not a use-after-free), but in some cases it is double-free, it is easier to locate double-free so I not mentioned use-after-free in #1007

The following is a screenshot of the test at that time.

core-dump double-free

I reproduced the test cases for double free as follows:

[root@localhost ~]# useradd u1
[root@localhost ~]# useradd u2
[root@localhost ~]# useradd u3
[root@localhost ~]# useradd u4
[root@localhost ~]# useradd u5
[root@localhost ~]# groupadd -U u1,u2,u3,u4,u5 g1
[root@localhost ~]# groupmod -n g2 -U u4,u3 g1
Segmentation fault

So this is a bug caused by an incorrect free and triggers two core dumps, one is double free and the other is use after free

gOo0se commented 4 months ago

Obviously in write_all it is a use-after-free (actually I never think it is not a use-after-free), but in some cases it is double-free, it is easier to locate double-free so I not mentioned use-after-free in #1007 The following is a screenshot of the test at that time. core-dump double-free

I reproduced the test cases for double free as follows:

[root@localhost ~]# useradd u1
[root@localhost ~]# useradd u2
[root@localhost ~]# useradd u3
[root@localhost ~]# useradd u4
[root@localhost ~]# useradd u5
[root@localhost ~]# groupadd -U u1,u2,u3,u4,u5 g1
[root@localhost ~]# groupmod -n g2 -U u4,u3 g1
Segmentation fault

So this is a bug caused by an incorrect free and triggers two core dumps, one is double free and the other is use after free

What makes the deferences just because of the number of users,but no matter how it does, wrong free when -n & -U caused the crash, so #1007 restrict the free condition to fix the bug.

alejandro-colomar commented 4 months ago

A simpler reproducer is here: