search

linux-audit / audit-kernel

GitHub mirror of the Linux Kernel's audit repository
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
Other
140 stars 37 forks source link

BUG: incorrect handling of creation of subdirectories #127

Open popov-nikita opened 4 years ago

popov-nikita commented 4 years ago

Hello, i've discovered particular issue where passing strings ending with '/' as directory name in mkdir syscall results in incorrect logging of directory name. Steps to reproduce behavior:

1) Create proof-of-concept directory: # mkdir /tmp/try.d 2) Add directory watch rule: # auditctl -S all -a always,exit -F dir=/tmp/try.d 3) Change cwd & create subdirectory as such:

# cd /tmp/try.d
# mkdir poc/

It's important to use '/' here at the end of mkdir's argument

After all these steps we will get the following logs:

type=SYSCALL msg=audit(1606129593.392:85): arch=c000003e syscall=83 success=yes exit=0 a0=7ffcec835401 a1=1ff a2=1 a3=55fe9c461010 items=5 ppid=13130 pid=13158 auid=1000 uid=1000 gid=1000 euid=1000 suid=1000 fsuid=1000 egid=1000 sgid=1000 fsgid=1000 tty=pts1 ses=3 comm="mkdir" exe="/bin/mkdir" key=(null)
type=CWD msg=audit(1606129593.392:85): cwd="/tmp/try.d"
type=PATH msg=audit(1606129593.392:85): item=0 name="/tmp/try.d" inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1606129593.392:85): item=1 name=(null) inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1606129593.392:85): item=2 name=(null) nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1606129593.392:85): item=3 name=(null) inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1606129593.392:85): item=4 name=(null) inode=17 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PROCTITLE msg=audit(1606129593.392:85): proctitle=6D6B64697200706F632F

Here the name of directory being created is completely missing and several empty PATH entries exist

In my opinion this is caused by the 'audit_compare_dname_path' function: https://github.com/linux-audit/audit-kernel/blob/ed1c04e9643cefd49de227830d0c63c68dcf2b75/kernel/auditfilter.c#L1301 Variable 'pathlen' is the length of 'path' which is original userland string (in our case - "poc/"). Variable 'dlen' is the length of directory name without any trailing '/'. In our case this is the length of "poc" string Variable 'parentlen' is computed by 'parent_len' function and should be 0. So given all that condition if (pathlen - parentlen != dlen) evaluates to true and strings don't match. This is what happens in __audit_inode_child function where we fail to find already existing parent.

rgbriggs commented 4 years ago

On 2020-11-23 03:53, Nikita Popov wrote:

Hello, i've discovered particular issue where passing strings ending with '/' as directory name in mkdir syscall results in incorrect logging of directory name.

(snip reproducer)

type=PATH msg=audit(1606129593.392:85): item=0 name="/tmp/try.d" inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0 type=PATH msg=audit(1606129593.392:85): item=1 name=(null) inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0 type=PATH msg=audit(1606129593.392:85): item=2 name=(null) nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0 type=PATH msg=audit(1606129593.392:85): item=3 name=(null) inode=16 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0 type=PATH msg=audit(1606129593.392:85): item=4 name=(null) inode=17 dev=00:2f mode=040755 ouid=1000 ogid=1000 rdev=00:00 nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0

Ok, agreed. I can reproduce this on f32. Items 0 and 4 should be the only ones there.

In my opinion this is caused by the 'audit_compare_dname_path' function: https://github.com/linux-audit/audit-kernel/blob/ed1c04e9643cefd49de227830d0c63c68dcf2b75/kernel/auditfilter.c#L1301 Variable 'pathlen' is the length of 'path' which is original userland string (in our case - "poc/"). Variable 'dlen' is the length of directory name without any trailing '/'. In our case this is the length of "poc" string Variable 'parentlen' is computed by 'parent_len' function and should be 0. So given all that condition if (pathlen - parentlen != dlen) evaluates to true and strings don't match. This is what happens in __audit_inode_child function where we fail to find already existing parent.

Thanks for the very clear reproducer and the sleuthing. This will need a closer look. It may be as simple as checking for a trailing '/' and treat it similarly. As another quick data point, "mkdir -p /tmp/try.d/poc/./" denial works as expected.