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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 14 Aug 2021 08:47:53 -0400
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Tianxing Zhang <anakinzhang96@...il.com>, zohar@...ux.ibm.com
Cc:     linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/1] ima: check control characters in policy path

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