[Applied] [PATCH 1/2] AppArmor: fix the upper bound check for the next/check table

[Brad Figg]

On 12/06/2010 11:49 AM, Tim Gardner wrote:
> On 12/03/2010 03:14 PM, John Johansen wrote:
>> BugLink: http://bugs.launchpad.net/bugs/581525
>>
>> SRU Justification (apparmor)
>>
>>      impact of the bug is medium for stable releases. There are two parts to
>>      this bug: the kernel side OOPSing when a the parser generates invalid
>>      tables, and the parser generating correct tables. The lucid kernel should
>>      receive the fix sometime in the future, but the userspace should also be
>>      fixed.
>>
>>      The kernel bug was a broken test in verifying the dfa next/check table
>>      size (so the userspace bug was not caught when it should have been). This
>>      means that it can at times reference beyond the dfa table (by at most 255
>>      entries).
>>
>>      The userspace bug is that the next/check table is not correctly padded
>>      with 0 entries, so that it is impossible to reference beyond the end of
>>      the table when in the states that use the end of the table for their
>>      references.
>>
>> The next/check table needs to be>= largest base value + 255 (byte range
>> being 0..255) to avoid any possible bounds violations.  Fix the test which
>> incorrectly was testing that the next/check table + 256<= base values.
>>
>> Signed-off-by: John Johansen<john.johansen at canonical.com>
>> ---
>>    security/apparmor/match.c |    4 +++-
>>    1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
>> index a3730e2..b456495 100644
>> --- a/security/apparmor/match.c
>> +++ b/security/apparmor/match.c
>> @@ -177,8 +177,10 @@ static int verify_dfa(struct aa_dfa *dfa, int flags)
>>    			if (DEFAULT_TABLE(dfa)[i]>= state_count)
>>    				goto out;
>>    			/* TODO: do check that DEF state recursion terminates */
>> -			if (BASE_TABLE(dfa)[i]>= trans_count + 256)
>> +			if (BASE_TABLE(dfa)[i] + 255>= trans_count) {
>> +				printk("AppArmor DFA next/check upper bounds error\n");
>>    				goto out;
>> +			}
>>    		}
>>
>>    		for (i = 0; i<   trans_count; i++) {
>
> Matches the upstream code.
>
> Acked-by: Tim Gardner<tim.gardner at canonical.com>
>

Acked-by: Brad Figg <brad.figg at canonical.com>

Applied to Lucid master-next.

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com