Insecure use of /dev/shm

[jwilk]

/dev/shm is world-writable, so you need to be careful when creating files or directories there. This silently succeeds when /dev/shm/pa is owned by a (potentially malicious) local user:

    tmpfile="/dev/shm/pa/$name.txt"
    tmpdir="$(dirname "$tmpfile")"
    mkdir -p "$tmpdir"

[biox]

ty for raising this @jwilk! :heart:

for my own curiosity: even if a malicious local user owns the parent dir, I'm not seeing how that causes a security issue. all pa edit files are still created as the effective user w/ a 077. I tested setting the setgid bit on a maliciously-owned dir, and couldn't figure out any way for a malicious user to read any data by simply owning the dir.

could you help me understand how dir ownership == data breach?

i did discover that by owning specific files, malicious users could gain access to password data. i patched this in 6b9c395 - the patch also resolves any malicious dir ownership concerns, i believe.

[jwilk]

could you help me understand how dir ownership == data breach?

On Linux, the attacker can do:

$ mkdir -m 777 /dev/shm/pa
$ setfacl -d -m "u:$LOGNAME:r" /dev/shm/pa

Then all files created in /dev/shm/pa will be readable to the attacker.

[biox]

Gotcha - thank you for that, I totally forgot about extended ACLs. I just pushed 0ea7ee1, which I've tested against the use-case you outlined & several others - I think it's safe. I'd appreciate if you could look it over & let me know if you think it's sound.

[jwilk]

It's not good.

First of all, denial of sevice is still possible. If the attacker created /dev/shm/pa, then nobody else (except root) could use pa edit.

Moreover, rm -rf /dev/shm/pa is advertised as a security measure, but it actually make things worse. It may delete your own /dev/shm/pa when it's still in use by another pa edit instance, possibly enabling worse-than-DoS attacks.

You should use mktemp -d /dev/shm/pa.XXXXXX (or something similar) for creating temporary directories.

[biox]

@jwilk ty for the feedback, again. i went through it this morning & i wound up simplifying pw_edit quite a bit - here's the latest version: https://github.com/biox/pa/blob/31a28dffd66a3f397d91f972b6326730317e9ee0/pa#L41-L77

I think that this protects against all of the classes of security issue you outline above, as well as the potential DoS attacks you pointed out.

Thoughts?

[jwilk]

6cd0cdbb81fec9ef7e006176655328c2153899b9 looks good to me.

[biox]

thx for the report & reviews :heart: i'll close this now.