search

linux-application-whitelisting / fapolicyd

File Access Policy Daemon
GNU General Public License v3.0
199 stars 56 forks source link

Fix trust_file_append function #171

Closed ZoltanFridrich closed 2 years ago

ZoltanFridrich commented 2 years ago

Variable int i before escaping patch was inside the loop and that was done deliberately. Within the escaping patch it has been moved outside of the loop which broke the trust_file_append function as the variable i is used within make_path_string function as both input and output. Moving it outside of the cycle caused that output of the previous iteration was used as an input in the current iteration.

Bug: lets say I have directory /opt/testdir/ that contains tesfile and subdir/testfile ... now I do fapolicyd-cli -f add /opt/testdir/ and when I check /etc/fapolicyd/fapolicyd.trust I see something like this:

/opt/testdir/testfile 0 sadfnsaldkjf....
/opt/testdir/subdir/testfile pt/testdir/subdir/testfile dfgnfskdjfhgber...

The issue is reproducible when you use fapolicyd-cli to add a directory with multiple contents into trust DB. Within this patch I moved the i variable back inside the loop which fixes the issue + I did some minor cleanup.

stevegrubb commented 2 years ago

Looking at the make_path_string function, I don't see why count is an int *. Nothing looks at the returned value. So, it probably makes more sense to change that parameter to int format and define 2 constants that determine which format gets used. Then do away with int i.

ZoltanFridrich commented 2 years ago

I just took make_path_string from file.c when I was implementing trust.d directory. I didn't bother changing it as I didn't understand why it was designed this strangely and it worked. But it is probably a good idea to change the arguments of the function and simplify it.