Skip to content

fix: remove usage of insecure tmpnam #3149

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

Merged
merged 4 commits into from
May 31, 2024
Merged

fix: remove usage of insecure tmpnam #3149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d4d71b4
fix: remove unsafe tmpnam usage
fzipi May 23, 2024
e9d0150
refactor: add acquire mutex function
fzipi May 29, 2024
54f531e
fix: add error logging
fzipi May 29, 2024
93aa06b
feat: consolidate into acquire_global_lock and export prototype
fzipi May 30, 2024
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
97 changes: 49 additions & 48 deletions apache2/modsecurity.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,49 @@ msc_engine *modsecurity_create(apr_pool_t *mp, int processing_mode) {
return msce;
}

int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp) {
apr_status_t rc;
apr_file_t *lock_name;
const char *temp_dir;
const char *filename;

// get platform temp dir
rc = apr_temp_dir_get(&temp_dir, mp);
if (rc != APR_SUCCESS) {
ap_log_perror(APLOG_MARK, APLOG_ERR, 0, NULL, "ModSecurity: Could not get temp dir");
return -1;
}

// use temp path template for lock files
char *path = apr_pstrcat(mp, temp_dir, GLOBAL_LOCK_TEMPLATE, NULL);

rc = apr_file_mktemp(&lock_name, path, 0, mp);
if (rc != APR_SUCCESS) {
ap_log_perror(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Could not create temporary file for global lock");
return -1;
}
// below func always return APR_SUCCESS
apr_file_name_get(&filename, lock_name);

rc = apr_global_mutex_create(&lock, filename, APR_LOCK_DEFAULT, mp);
if (rc != APR_SUCCESS) {
ap_log_perror(APLOG_MARK, APLOG_ERR, 0, NULL, " ModSecurity: Could not create global mutex");
return -1;
}
#if !defined(MSC_TEST)
#ifdef __SET_MUTEX_PERMS
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2
rc = ap_unixd_set_global_mutex_perms(lock);
#else
rc = unixd_set_global_mutex_perms(lock);
#endif
if (rc != APR_SUCCESS) {
return -1;
}
#endif /* SET_MUTEX_PERMS */
#endif /* MSC_TEST */
return APR_SUCCESS;
}
/**
* Initialise the modsecurity engine. This function must be invoked
* after configuration processing is complete as Apache needs to know the
Expand All @@ -132,7 +175,7 @@ int modsecurity_init(msc_engine *msce, apr_pool_t *mp) {

msce->auditlog_lock = msce->geo_lock = NULL;
#ifdef GLOBAL_COLLECTION_LOCK
msce->geo_lock = NULL;
msce->dbm_lock = NULL;
#endif

/**
Expand All @@ -145,65 +188,23 @@ int modsecurity_init(msc_engine *msce, apr_pool_t *mp) {
#ifdef WITH_CURL
curl_global_init(CURL_GLOBAL_ALL);
#endif
/* Serial audit log mutext */
tmpnam(auditlog_lock_name);
rc = apr_global_mutex_create(&msce->auditlog_lock, auditlog_lock_name, APR_LOCK_DEFAULT, mp);
/* Serial audit log mutex */
rc = acquire_global_lock(msce->auditlog_lock, mp);
if (rc != APR_SUCCESS) {
//ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, "mod_security: Could not create modsec_auditlog_lock");
//return HTTP_INTERNAL_SERVER_ERROR;
return -1;
}

#if !defined(MSC_TEST)
#ifdef __SET_MUTEX_PERMS
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2
rc = ap_unixd_set_global_mutex_perms(msce->auditlog_lock);
#else
rc = unixd_set_global_mutex_perms(msce->auditlog_lock);
#endif
if (rc != APR_SUCCESS) {
// ap_log_error(APLOG_MARK, APLOG_ERR, rc, s, "mod_security: Could not set permissions on modsec_auditlog_lock; check User and Group directives");
// return HTTP_INTERNAL_SERVER_ERROR;
return -1;
}
#endif /* SET_MUTEX_PERMS */

tmpnam(geo_lock_name);
rc = apr_global_mutex_create(&msce->geo_lock, geo_lock_name, APR_LOCK_DEFAULT, mp);
rc = acquire_global_lock(msce->geo_lock, mp);
if (rc != APR_SUCCESS) {
return -1;
}

#ifdef __SET_MUTEX_PERMS
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2
rc = ap_unixd_set_global_mutex_perms(msce->geo_lock);
#else
rc = unixd_set_global_mutex_perms(msce->geo_lock);
#endif
if (rc != APR_SUCCESS) {
return -1;
}
#endif /* SET_MUTEX_PERMS */

#ifdef GLOBAL_COLLECTION_LOCK
tmpnam(dbm_lock_name);
rc = apr_global_mutex_create(&msce->dbm_lock, dbm_lock_name, APR_LOCK_DEFAULT, mp);
rc = acquire_global_lock(&msce->dbm_lock, mp);
if (rc != APR_SUCCESS) {
return -1;
}

#ifdef __SET_MUTEX_PERMS
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 2
rc = ap_unixd_set_global_mutex_perms(msce->dbm_lock);
#else
rc = unixd_set_global_mutex_perms(msce->dbm_lock);
#endif
if (rc != APR_SUCCESS) {
return -1;
}
#endif /* SET_MUTEX_PERMS */
#endif
#endif
#endif /* GLOBAL_COLLECTION_LOCK */

return 1;
}
Expand Down
9 changes: 4 additions & 5 deletions apache2/modsecurity.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,7 @@ typedef struct msc_parm msc_parm;

#define FATAL_ERROR "ModSecurity: Fatal error (memory allocation or unexpected internal error)!"

static char auditlog_lock_name[L_tmpnam];
static char geo_lock_name[L_tmpnam];
#ifdef GLOBAL_COLLECTION_LOCK
static char dbm_lock_name[L_tmpnam];
#endif
#define GLOBAL_LOCK_TEMPLATE "/modsec-lock-tmp.XXXXXX"

extern DSOLOCAL char *new_server_signature;
extern DSOLOCAL char *real_server_signature;
Expand Down Expand Up @@ -709,6 +705,9 @@ struct msc_parm {
int pad_2;
};

/* Reusable functions */
int acquire_global_lock(apr_global_mutex_t *lock, apr_pool_t *mp);

/* Engine functions */

msc_engine DSOLOCAL *modsecurity_create(apr_pool_t *mp, int processing_mode);
Expand Down