Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix audit records #1188

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix audit records #1188

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
038ff23
Fix audit records
stevegrubb Jan 18, 2025
a82a01a
Updates based on feedback
stevegrubb Jan 21, 2025
88e145a
Another round of updates
stevegrubb Jan 22, 2025
0d86d7c
Invert streq value
stevegrubb Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions lib/audit_help.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <libaudit.h>
#include <errno.h>
#include <stdio.h>
#include <alloca.h>

#include "attr.h"
#include "prototypes.h"
Expand All @@ -47,18 +48,16 @@
/*
* This takes a string and replaces the old character with the new.
*/
static char *strreplace (char *str, char old, char new)
static inline char *strtr (char *str, char old, char new)
alejandro-colomar marked this conversation as resolved.
Show resolved Hide resolved
{
if (str == NULL) {
return NULL;
}

char *p = str;
while (*p) {
for (char *p = str; *p; p++) {
alejandro-colomar marked this conversation as resolved.
Show resolved Hide resolved
if (*p == old) {
*p = new;
}
p++;
}
return str;
}
Expand All @@ -85,13 +84,14 @@
/*
* The audit system needs white space in the op field to
* be replaced with dashes so that parsers get the whole
* field.
* field. Not all C libraries have strdupa.
*/
char *fixed_op = strreplace (strdup (op), ' ', '-');
char *tmp_op = alloca (strlen (op) + 1);
Copy link
Collaborator

@alejandro-colomar alejandro-colomar Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strlen() + 1 doesn't enforce a string literal.

Using alloca(3) with arbitrary input is dangerous.

Please add a macro that validates the input and calls alloca().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are hard-coded strings embedded in the shadow-utils code. They cannot be attacker influenced.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but we might forget and change some of those in the future. That's why I wanted a STRDUPA() macro that enforces that the input is a string literal.

strcpy (tmp_op, op);
Fixed Show fixed Hide fixed
char *fixed_op = strtr (tmp_op, ' ', '-');
alejandro-colomar marked this conversation as resolved.
Show resolved Hide resolved
audit_log_acct_message (audit_fd, type, NULL,
fixed_op ? fixed_op : op, name,
id, NULL, NULL, NULL, result);
free (fixed_op);
fixed_op, name, id,
NULL, NULL, NULL, result);
}
}

Expand Down
Loading