Segmentation fault when generating policy with gradm

[basegat]

Having collected enough usage data for my system I tried to run it through gradm's learning mode and noticed that the program exited after the message "Beginning full learning role reduction...". Dmesg revealed that gradm had segfaulted and after running it within debugger the culprit was found to be the function "free_entire_user_node_list" in gradm_newlearn.c on line 402. On a closer look I noticed that during the iteration of the userlist the list pointer "tmpuser" is incorrectly advanced within the for_each_list_entry-loop ultimately resulting in segmentation fault:

Code: Select all

for_each_list_entry(tmpuser, *userlist) {
      freeuser = tmpuser;
      tmpuser = tmpuser->next;
      free(freeuser->rolename);
      gr_free(freeuser);
   }


Commenting out tmpuser = tmpuser->next seems to fix the issue and after that gradm generates the policy file flawlessly.

Steps to reproduce:
1. Install gradm-3.1-201507041117
2. Create a learning log somewhere with following example content:

Code: Select all

default   68   0   100   /usr/bin/sudo   /   1   1   /etc/login.defs   16   0.0.0.0
default   68   0   100   /usr/bin/sudo   /   1   1   /etc/login.defs   17   0.0.0.0
default   68   0   100   /usr/bin/sudo   /   1   1   /etc/environment   16   0.0.0.0
default   68   0   100   /usr/bin/sudo   /   1   1   /etc/environment   17   0.0.0.0
default   68   1000   100   /usr/bin/tmux   /   1   1   /proc   16   0.0.0.0
default   68   1005   100   /usr/bin/sudo   /   1   1   /usr/lib/libz.so.1.2.8   8   0.0.0.0
default   68   1005   100   /usr/bin/sudo   /   1   1   /usr/lib/libsasl2.so.3.0.0   17   0.0.0.0


3. Run it through the full system learning mode: gradm -F -L path_to_log -O output
4. Gradm outputs the following and exits:

Code: Select all

Beginning full learning 1st pass...done.
Beginning full learning role reduction...


[spender]

Thanks for the report -- I just uploaded a new tarball that fixes it. While your fix will likely work in practice, technically it introduces a use-after-free on 'tmpuser'. The correct fix is to use our for_each_removable_list_entry() macro which I did in the uploaded fix.

Thanks again for the detailed report!
-Brad