Skip to content

Commit c3b0042

Browse files
committed
Fix some static analysis findings
struct audit_message embeds a netlink header followed by a fixed-size payload buffer, so copying more than sizeof(req.data) bytes into the stack object will clobber adjacent state. The updated sender now rejects requests whose payload exceeds that buffer and makes sure the padded wire size stays within the kernel’s maximum before copying the user data into the in-struct array, eliminating the unchecked write Coverity spotted. We still populate nlmsg_len with NLMSG_SPACE(size), so the layout expected by the historical macros is preserved while giving the analyzer an obvious bound check. During a live reconfigure, the new settings structure is torn down once the merge finishes. If we simply assigned oconf->plugin_dir = nconf->plugin_dir, the old configuration would wind up pointing at memory that destroy_config(nconf) later frees, which is exactly what Coverity warned about. The revised code duplicates any new path into storage owned by the running configuration, frees the superseded value, and clears whichever pointer the temporary configuration no longer owns so the final cleanup cannot free it twice. That removes the aliasing path Coverity described. Bad-login reporting used to stuff whatever pointer auparse returned into a temporary stack lnode and then pass it to report_session, which blindly printed term+5 when the name started with /dev/. If term was the short literal "?", advancing five bytes ran off the end of the two-byte buffer Coverity modeled. The new logic normalizes the pointer before printing—falling back to "?" when the field is missing and only skipping the /dev/ prefix when the string is actually long enough—so nothing dereferences past the available characters.
1 parent b83c84e commit c3b0042

File tree

3 files changed

+39
-9
lines changed

3 files changed

+39
-9
lines changed

lib/netlink.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ int __audit_send(int fd, int type, const void *data, unsigned int size, int *seq
200200
return -errno;
201201
}
202202

203+
if (size > sizeof(req.data)) {
204+
errno = EINVAL;
205+
return -errno;
206+
}
207+
203208
if (NLMSG_SPACE(size) > MAX_AUDIT_MESSAGE_LENGTH) {
204209
errno = EINVAL;
205210
return -errno;
@@ -217,7 +222,7 @@ int __audit_send(int fd, int type, const void *data, unsigned int size, int *seq
217222
req.nlh.nlmsg_flags = NLM_F_REQUEST|NLM_F_ACK;
218223
req.nlh.nlmsg_seq = sequence;
219224
if (size && data)
220-
memcpy(NLMSG_DATA(&req.nlh), data, size);
225+
memcpy(req.data, data, size);
221226
memset(&addr, 0, sizeof(addr));
222227
addr.nl_family = AF_NETLINK;
223228
addr.nl_pid = 0;

src/auditd-event.c

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,11 +1574,29 @@ static void reconfigure(struct auditd_event *e)
15741574
oconf->q_depth = nconf->q_depth;
15751575
oconf->overflow_action = nconf->overflow_action;
15761576
oconf->max_restarts = nconf->max_restarts;
1577-
if (oconf->plugin_dir != nconf->plugin_dir ||
1578-
(oconf->plugin_dir && nconf->plugin_dir &&
1579-
strcmp(oconf->plugin_dir, nconf->plugin_dir) != 0)) {
1577+
if (nconf->plugin_dir) {
1578+
if (!oconf->plugin_dir ||
1579+
strcmp(oconf->plugin_dir,
1580+
nconf->plugin_dir) != 0) {
1581+
char *tmp = strdup(nconf->plugin_dir);
1582+
1583+
if (tmp == NULL)
1584+
audit_msg(LOG_ERR,
1585+
"Cannot duplicate plugin_dir in reconfigure");
1586+
else {
1587+
free(oconf->plugin_dir);
1588+
oconf->plugin_dir = tmp;
1589+
}
1590+
}
1591+
} else if (oconf->plugin_dir) {
15801592
free(oconf->plugin_dir);
1581-
oconf->plugin_dir = nconf->plugin_dir;
1593+
oconf->plugin_dir = NULL;
1594+
}
1595+
if (nconf->plugin_dir == oconf->plugin_dir)
1596+
nconf->plugin_dir = NULL;
1597+
else {
1598+
free(nconf->plugin_dir);
1599+
nconf->plugin_dir = NULL;
15821600
}
15831601

15841602
/* At this point we will work on the items that are related to

tools/aulast/aulast.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ void usage(void)
5353
static void report_session(lnode* cur)
5454
{
5555
int notime = 0;
56+
const char *term;
57+
const char *term_to_print;
5658

5759
// Don't list failed logins
5860
if (cur == NULL)
@@ -71,10 +73,15 @@ static void report_session(lnode* cur)
7173
} else
7274
printf("%-8.u ", cur->auid);
7375

74-
if (strncmp("/dev/", cur->term, 5) == 0)
75-
printf("%-12.12s ", cur->term+5);
76-
else
77-
printf("%-12.12s ", cur->term);
76+
term = cur->term ? cur->term : "?";
77+
term_to_print = term;
78+
if (strncmp(term, "/dev/", 5) == 0) {
79+
if (strlen(term) > 5)
80+
term_to_print = term + 5;
81+
else
82+
term_to_print = "";
83+
}
84+
printf("%-12.12s ", term_to_print);
7885
printf("%-16.16s ", cur->host ? cur->host : "?");
7986
printf("%-16.16s ", ctime(&cur->start));
8087
switch(cur->status)

0 commit comments

Comments
 (0)