[apparmor] [PATCH 03/16] AppArmor: Fix underflow in xindex calculation

[Kees Cook]

On Wed, Feb 22, 2012 at 12:44:54PM -0800, John Johansen wrote:
> On 02/22/2012 12:27 PM, Kees Cook wrote:
> > On Wed, Feb 22, 2012 at 09:10:28AM -0800, John Johansen wrote:
> >> If the xindex value stored in the accept tables is 0, the extraction of
> >> that value will result in an underflow (0 - 4).
> >>
> >> In properly compiled policy this should not happen for file rules but
> >> it may be possible for other rule types in the future.
> >>
> >> To exploit this underflow a user would have to be able to load a corrupt
> >> policy, which requires CAP_MAC_ADMIN, overwrite system policy in kernel
> >> memory or know of a compiler error resulting in the flaw being present
> >> for loaded policy (no such flaw is known at this time).
> >>
> >> Signed-off-by: John Johansen <john.johansen at canonical.com>
> >> ---
> >>  security/apparmor/include/file.h |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
> >> index ab8c6d8..f98fd47 100644
> >> --- a/security/apparmor/include/file.h
> >> +++ b/security/apparmor/include/file.h
> >> @@ -117,7 +117,7 @@ static inline u16 dfa_map_xindex(u16 mask)
> >>  		index |= AA_X_NAME;
> >>  	} else if (old_index == 3) {
> >>  		index |= AA_X_NAME | AA_X_CHILD;
> >> -	} else {
> >> +	} else if (old_index) {
> >>  		index |= AA_X_TABLE;
> >>  		index |= old_index - 4;
> >>  	}
> > 
> > What about the cases where old_index < 4, but != 0?
> > 
> look above cases 1, 2, and 3 are covered by the if blocks
> eg.
> 
> } else if (old_index == 3) {
> 	index |= AA_X_NAME | AA_X_CHILD;

Ah, right. Okay. Missed that bit. Thanks!

Signed-off-by: Kees Cook <kees at ubuntu.com>

-- 
Kees Cook