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

Originally reproduced here:

#1007

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.

See #1007 (comment)

The crash can be traced to this path:

shadow/src/groupmod.c

Line 857 in 69f74db

close_files ();

shadow/src/groupmod.c

Line 470 in 69f74db

if (gr_close () == 0) {

shadow/lib/commonio.c

Line 981 in 69f74db

if (write_all (db) != 0) {

shadow/lib/commonio.c

Line 879 in 69f74db

if (db->ops->put (eptr, db->fp) != 0) {

shadow/lib/groupio.c

Line 64 in 69f74db

|| (valid_field (gr->gr_name, ":\n") == -1)

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.

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.

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.

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.

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

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.

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.

A simpler reproducer is here:

#1007 (comment)