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 - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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 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.

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

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.