Page MenuHomeFreeBSD

auditon(2): Fix the misplacement of negation symbol in A_SETPOLICY command
ClosedPublic

Authored by aniketp on Jul 11 2018, 11:56 AM.

Details

Summary

The code snippet for auditon(2)'s A_{GET/SET}POLICY command confirms whether the user
submitted only the allowed values for A_{SET/GET}POLICY in the following way:

if (udata.au_policy & ~(AUDIT_CNT|AUDIT_AHLT|AUDIT_ARGV|AUDIT_ARGE))
	return (EINVAL);

That is: If a bit other than the 4 mentioned above is submitted, we'll get EINVAL.

However, The negation bit is misplaced for "udata.au_policy64" case:

if (udata.au_policy & (~AUDIT_CNT|AUDIT_AHLT|AUDIT_ARGV|AUDIT_ARGE))
	return (EINVAL);

This returns EINVAL almost everytime. The only occasion when I got a successful invocation
was passing 0 as the flags, which is not recommended if the system already has some of the
flags set.

Note: The issue was noticed when it failed the regression tests for auditon(2) for
A_SETPOLICY flag.

Test Plan

To reproduce the issue, try this piece of code. If your system has any of the flags set,
you'll probably get EINVAL too.

#include <bsm/audit.h>
#include <stdio.h>

void main(){
        uint64_t auditpolicy;
        auditon(A_GETPOLICY, &auditpolicy, sizeof(auditpolicy));
        printf("retrieved policy = %d\n", auditpolicy);
       
        int retval = auditon(A_SETPOLICY, &auditpolicy, sizeof(auditpolicy));
        if (retval < 0)
                perror("setpolicy");
}

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

aniketp created this revision.Jul 11 2018, 11:56 AM
aniketp retitled this revision from Fix AUDIT_CNT bit negation's misplacement in A_SETPOLICY:auditon(2) to auditon(2): Fix the misplacement of negation symbol in A_SETPOLICY command.Jul 11 2018, 11:57 AM
asomers requested changes to this revision.Jul 11 2018, 2:44 PM

This looks like a real bug, but your reproduction code is wrong. You're passing as the third argument sizeof(&auditpolicy), which is the size of a pointer to the policy variable, not the size of the policy variable itself. To correctly reproduce this, you'll have to fix the sizeof and use a uint64_t argument instead of an int.

This revision now requires changes to proceed.Jul 11 2018, 2:44 PM
cem accepted this revision.Jul 11 2018, 4:46 PM
cem added a subscriber: cem.

Actual patch and commit message LGTM. As Alan points out, your test case may be getting EINVAL from having the wrong size, not due to flags.

aniketp edited the test plan for this revision. (Show Details)Jul 13 2018, 12:02 PM

I guess in one of the later tests, I'll have to fix this too: tests/sys/audit/administrative.c#L389

asomers accepted this revision.Jul 13 2018, 2:17 PM
This revision is now accepted and ready to land.Jul 13 2018, 2:17 PM
This revision was automatically updated to reflect the committed changes.