[PATCH 0/1] ima: check control characters in policy path

[PATCH 0/1] ima: check control characters in policy path

[Tianxing Zhang]

Hi,

IMA policy can be updated with /sys/kernel/security/ima/policy interface when
CONFIG_IMA_WRITE_POLICY is set. However, kernel does not check the file path
carefully. It only checks if the path has '/' prefix.

When a policy file path contains control characters like '\r' or '\b',
invalid error messages can be printed to overwrite system messages.

For example:

$ echo -e "/\rtest invalid path: ddddddddddddddddddddd" > /sys/kernel/security/ima/policy
$ dmesg
test invalid path: ddddddddddddddddddddd (-2) 

After adding this patch, we'll be able to throw out error message:

$ echo -e "/\rtest invalid path: ddddddddddddddddddddd" > /sys/kernel/security/ima/policy
-bash: echo: write error: Invalid argument
$ dmesg
[   11.684004] ima: invalid path (control characters are not allowed)
[   11.684071] ima: policy update failed

Any suggestions would be appreciated, thank you.

Tianxing Zhang (1):
  ima: check control characters in policy file path

 security/integrity/ima/ima_fs.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.25.1

Re: [PATCH 0/1] ima: check control characters in policy path

[James Bottomley]

On Sat, 2021-08-14 at 16:13 +0800, Tianxing Zhang wrote:
> Hi,
> 
> IMA policy can be updated with /sys/kernel/security/ima/policy
> interface when CONFIG_IMA_WRITE_POLICY is set. However, kernel does
> not check the file path carefully. It only checks if the path has '/'
> prefix.
> 
> When a policy file path contains control characters like '\r' or
> '\b', invalid error messages can be printed to overwrite system
> messages.

This doesn't sound like a good idea: filesystems accept control
characters in names, so the IMA file policy has to be able to specify
them.  We can debate whether filesystems should do this, but while they
do IMA has to as well.

> For example:
> 
> $ echo -e "/\rtest invalid path: ddddddddddddddddddddd" >
> /sys/kernel/security/ima/policy
> $ dmesg
> test invalid path: ddddddddddddddddddddd (-2) 
> 
> After adding this patch, we'll be able to throw out error message:
> 
> $ echo -e "/\rtest invalid path: ddddddddddddddddddddd" >
> /sys/kernel/security/ima/policy
> -bash: echo: write error: Invalid argument
> $ dmesg
> [   11.684004] ima: invalid path (control characters are not allowed)
> [   11.684071] ima: policy update failed
> 
> Any suggestions would be appreciated, thank you.

I don't quite understand what you think the problem is.  Only root can
write IMA policies so no-one other than a legitimate administrator can
use bogus paths like the above.  If the problem is producing a bogus
log message, we do have several IMA messages that print out
measured/appraised file names ... they would be vulnerable to this
since a generic user could have created them with control character
containg file names, and your proposed patch wouldn't fix that.

Wouldn't a better solution be to have a file name print that expands
the unprintable characters?

James