Skip to content

Commit 2f1ac7a

Browse files
committed
Code cleanup
Initialized the netlink sender address structure before recvfrom() so the reply path no longer reads an uninitialized sockaddr_nl field during spoof checks. Hardened auparse initialization and event iteration by removing the buffer-array dead nested assignment pattern and adding defensive checks around au->au_lo / lazy event-list allocation. Tightened x2c() so it bails out when the caller does not provide two usable bytes, avoiding the scan-build path that treated the hex input as undefined. Removed scan-build-reported dead assignments in syscall normalization by calling set_file_object() directly for the special fsetxattr, fchmod, and fchown cases, and by dropping the unused rc assignment before the "key" lookup.
1 parent aaabb6e commit 2f1ac7a

5 files changed

Lines changed: 33 additions & 14 deletions

File tree

auparse/auparse.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -524,9 +524,10 @@ auparse_state_t *auparse_init(ausource_t source, const void *b)
524524
if (bb == NULL)
525525
goto bad_exit;
526526
size = 0;
527-
for (n = 0; (buf = bb[n]); n++) {
528-
len = strlen(bb[n]);
529-
if (bb[n][len-1] != '\n') {
527+
for (n = 0; bb[n]; n++) {
528+
buf = bb[n];
529+
len = strlen(buf);
530+
if (buf[len-1] != '\n') {
530531
size += len + 1;
531532
} else {
532533
size += len;
@@ -1487,6 +1488,11 @@ static int au_auparse_next_event(auparse_state_t *au)
14871488
event_list_t *l;
14881489
au_event_t e;
14891490

1491+
if (au == NULL || au->au_lo == NULL) {
1492+
errno = EINVAL;
1493+
return -1;
1494+
}
1495+
14901496
/*
14911497
* Deal with Python memory management issues where it issues a
14921498
* auparse_destroy() call after an auparse_init() call but then wants
@@ -1500,8 +1506,11 @@ static int au_auparse_next_event(auparse_state_t *au)
15001506
#ifdef LOL_EVENTS_DEBUG01
15011507
if (debug) printf("Creating lol array\n");
15021508
#endif /* LOL_EVENTS_DEBUG01 */
1503-
au_lol_create(au->au_lo);
1509+
if (au_lol_create(au->au_lo) == NULL)
1510+
return -1;
15041511
}
1512+
if (au->au_lo->array == NULL)
1513+
return -1;
15051514

15061515
/*
15071516
* First see if we have any empty events but with an allocated event

auparse/interpret.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ static unsigned char x2c(const unsigned char *buf)
148148
const char *ptr;
149149
unsigned char total=0;
150150

151+
if (buf == NULL || buf[0] == 0 || buf[1] == 0)
152+
return 0;
153+
151154
ptr = strchr(AsciiArray, (char)toupper(buf[0]));
152155
if (ptr)
153156
total = (unsigned char)(((ptr-AsciiArray) & 0x0F)<<4);

auparse/lru.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,19 +297,26 @@ static void free_qnode(Queue *queue, QNode *node)
297297

298298
static void evict_lru(Queue *queue)
299299
{
300+
QNode *node;
301+
300302
if (queue_is_empty(queue))
301303
return;
302-
free_qnode(queue, queue->end);
304+
305+
node = queue->end;
306+
free_qnode(queue, node);
303307
queue->evictions++;
304308
}
305309

306310
// Remove from the end of the queue
307311
static void dequeue(Queue *queue)
308312
{
313+
QNode *node;
314+
309315
if (queue_is_empty(queue))
310316
return;
311317

312-
free_qnode(queue, queue->end);
318+
node = queue->end;
319+
free_qnode(queue, node);
313320
}
314321

315322

auparse/normalize.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -634,24 +634,25 @@ static int normalize_syscall(auparse_state_t *au, const char *syscall)
634634
act = "changed-file-attributes-of";
635635
D.thing.what = NORM_WHAT_FILE; // this gets overridden
636636
if (strcmp(syscall, "fsetxattr") == 0)
637-
offset = -1;
638-
set_file_object(au, offset);
637+
set_file_object(au, -1);
638+
else
639+
set_file_object(au, offset);
639640
simple_file_attr(au);
640641
break;
641642
case NORM_FILE_CHPERM:
642643
act = "changed-file-permissions-of";
643644
D.thing.what = NORM_WHAT_FILE; // this gets overridden
644645
if (strcmp(syscall, "fchmod") == 0)
645-
offset = -1;
646+
set_file_object(au, -1);
647+
else
648+
set_file_object(au, offset);
646649
collect_perm_obj2(au, syscall);
647-
set_file_object(au, offset);
648650
simple_file_attr(au);
649651
break;
650652
case NORM_FILE_CHOWN:
651653
act = "changed-file-ownership-of";
652654
D.thing.what = NORM_WHAT_FILE; // this gets overridden
653655
if (strcmp(syscall, "fchown") == 0) {
654-
offset = -1;
655656
collect_own_obj2(au, syscall);
656657
/* fchown has no cwd or path information */
657658
} else {
@@ -978,7 +979,7 @@ static int normalize_syscall(auparse_state_t *au, const char *syscall)
978979
default:
979980
{
980981
const char *k;
981-
rc = auparse_first_record(au);
982+
auparse_first_record(au);
982983
k = auparse_find_field(au, "key");
983984
if (k && strcmp(k, "(null)")) {
984985
act = "triggered-audit-rule";

lib/netlink.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ void audit_close(int fd)
7777
int audit_get_reply(int fd, struct audit_reply *rep, reply_t block, int peek)
7878
{
7979
int len;
80-
struct sockaddr_nl nladdr;
80+
struct sockaddr_nl nladdr = { 0 };
8181
socklen_t nladdrlen = sizeof(nladdr);
8282

8383
if (fd < 0)
@@ -299,4 +299,3 @@ static int check_ack(int fd)
299299
}
300300
return 0;
301301
}
302-

0 commit comments

Comments
 (0)