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.

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

Use long long instead of int for comparison, increment, decrement operations and persistent collections #2134

Open
wants to merge 1 commit into
base: v2/master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 20 additions & 8 deletions apache2/persist_dbm.c
Original file line number Diff line number Diff line change
Expand Up @@ -528,22 +528,34 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
if (orig_var != NULL) {
const msc_string *stored_var = (const msc_string *)apr_table_get(stored_col, var->name);
if (stored_var != NULL) {
int origval = atoi(orig_var->value);
int ourval = atoi(var->value);
int storedval = atoi(stored_var->value);
int delta = ourval - origval;
int newval = storedval + delta;
long long origval = atoll(orig_var->value);
long long ourval = atoll(var->value);
long long storedval = atoll(stored_var->value);
long long delta = ourval - origval;
long long newval = storedval + delta;

if (newval < 0) newval = 0; /* Counters never go below zero. */

var->value = apr_psprintf(msr->mp, "%d", newval);
/* The printf functions provided by Apache do not understand the %lld format.
* Therefore, we convert value to a string first. */
char newval_str[23] = {'\0'};
sprintf(newval_str, "%lld", newval);
var->value = apr_psprintf(msr->mp, "%s", newval_str);
var->value_len = strlen(var->value);

if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "collection_store: Delta applied for %s.%s %d->%d (%d): %d + (%d) = %d [%s,%d]",
char origval_str[23] = {'\0'};
sprintf(origval_str, "%lld", origval);
char ourval_str[23] = {'\0'};
sprintf(ourval_str, "%lld", ourval);
char delta_str[23] = {'\0'};
sprintf(delta_str, "%lld", delta);
char storedval_str[23] = {'\0'};
sprintf(storedval_str, "%lld", storedval);
msr_log(msr, 9, "collection_store: Delta applied for %s.%s %s->%s (%s): %s + (%s) = %s [%s,%d]",
log_escape_ex(msr->mp, var_name->value, var_name->value_len),
log_escape_ex(msr->mp, var->name, var->name_len),
origval, ourval, delta, storedval, delta, newval, var->value, var->value_len);
origval_str, ourval_str, delta_str, storedval_str, delta_str, newval_str, var->value, var->value_len);
}
}
}
Expand Down
49 changes: 33 additions & 16 deletions apache2/re_actions.c
Original file line number Diff line number Diff line change
Expand Up @@ -1606,7 +1606,7 @@ apr_status_t msre_action_setvar_execute(modsec_rec *msr, apr_pool_t *mptmp,
/* Relative change. */
msc_string *rec = NULL;
msc_string *val = apr_palloc(msr->mp, sizeof(msc_string));
int value = 0;
long long value = 0;

if (val == NULL) {
msr_log(msr, 1, "Failed to allocate space to expand value macros");
Expand All @@ -1620,11 +1620,11 @@ apr_status_t msre_action_setvar_execute(modsec_rec *msr, apr_pool_t *mptmp,
rec->name = apr_pstrdup(msr->mp, var_name);
rec->name_len = strlen(rec->name);
value = 0;
rec->value = apr_psprintf(msr->mp, "%d", value);
rec->value = apr_psprintf(msr->mp, "%d", 0);
rec->value_len = strlen(rec->value);
}
else {
value = atoi(rec->value);
value = atoll(rec->value);
}

/* Record the original value before we change it */
Expand All @@ -1641,15 +1641,23 @@ apr_status_t msre_action_setvar_execute(modsec_rec *msr, apr_pool_t *mptmp,
var_value = val->value;

if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Relative change: %s=%d%s", var_name, value, var_value);
/* The printf functions provided by Apache do not understand the %lld format.
* Therefore, we convert value to a string first. */
char value_str[23] = {'\0'};
sprintf(value_str, "%lld", value);
msr_log(msr, 9, "Relative change: %s=%s%s", var_name, value_str, var_value);
}

/* Change value. */
value += atoi(var_value);
value += atoll(var_value);
if (value < 0) value = 0; /* Counters never go below zero. */

/* Put the variable back. */
rec->value = apr_psprintf(msr->mp, "%d", value);
/* The printf functions provided by Apache do not understand the %lld format.
* Therefore, we convert value to a string first. */
char value_str[23] = {'\0'};
sprintf(value_str, "%lld", value);
rec->value = apr_psprintf(msr->mp, "%s", value_str);
rec->value_len = strlen(rec->value);
apr_table_setn(target_col, rec->name, (void *)rec);

Expand Down Expand Up @@ -1830,7 +1838,7 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t
apr_table_t *target_col = NULL;
msc_string *var = NULL, *var_last_update_time = NULL;
apr_time_t last_update_time, current_time;
long current_value, new_value;
long long current_value, new_value;

/* Extract the name and the value. */
/* IMP1 We have a function for this now, parse_name_eq_value? */
Expand Down Expand Up @@ -1901,7 +1909,7 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t
}
return 0;
}
current_value = atoi(var->value);
current_value = atoll(var->value);

/* Find the last update time (of the collection). */
var_last_update_time = (msc_string *)apr_table_get(target_col, "LAST_UPDATE_TIME");
Expand All @@ -1928,27 +1936,36 @@ static apr_status_t msre_action_deprecatevar_execute(modsec_rec *msr, apr_pool_t
* time elapsed since the last update.
*/
new_value = current_value -
(atol(var_value) * ((current_time - last_update_time) / atol(s)));
(atoll(var_value) * ((current_time - last_update_time) / atoll(s)));
if (new_value < 0) new_value = 0;

/* The printf functions provided by Apache do not understand the %lld format.
* Therefore, we convert value to a string first. */
char new_value_str[23] = {'\0'};
sprintf(new_value_str, "%lld", new_value);

/* Only change the value if it differs. */
if (new_value != current_value) {
var->value = apr_psprintf(msr->mp, "%ld", new_value);
var->value = apr_psprintf(msr->mp, "%s", new_value_str);
var->value_len = strlen(var->value);

if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "Deprecated variable \"%s.%s\" from %ld to %ld (%" APR_TIME_T_FMT " seconds since "
char current_value_str[23] = {'\0'};
sprintf(current_value_str, "%lld", current_value);
msr_log(msr, 4, "Deprecated variable \"%s.%s\" from %s to %s (%" APR_TIME_T_FMT " seconds since "
"last update).", log_escape(msr->mp, col_name), log_escape(msr->mp, var_name),
current_value, new_value, (apr_time_t)(current_time - last_update_time));
current_value_str, new_value_str, (apr_time_t)(current_time - last_update_time));
}

apr_table_set(msr->collections_dirty, col_name, "1");
} else {
if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Not deprecating variable \"%s.%s\" because the new value (%ld) is "
"the same as the old one (%ld) (%" APR_TIME_T_FMT " seconds since last update).",
log_escape(msr->mp, col_name), log_escape(msr->mp, var_name), current_value,
new_value, (apr_time_t)(current_time - last_update_time));
char current_value_str[23] = {'\0'};
sprintf(current_value_str, "%lld", current_value);
msr_log(msr, 9, "Not deprecating variable \"%s.%s\" because the new value (%s) is "
"the same as the old one (%s) (%" APR_TIME_T_FMT " seconds since last update).",
log_escape(msr->mp, col_name), log_escape(msr->mp, var_name), current_value_str,
new_value_str, (apr_time_t)(current_time - last_update_time));
}
}

Expand Down
40 changes: 20 additions & 20 deletions apache2/re_operators.c
Original file line number Diff line number Diff line change
Expand Up @@ -4418,7 +4418,7 @@ static int msre_op_eq_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char **error_msg)
{
msc_string str;
int left, right;
long long left, right;
char *target = NULL;

if (error_msg == NULL) return -1;
Expand All @@ -4436,15 +4436,15 @@ static int msre_op_eq_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,

target = apr_pstrmemdup(msr->mp, var->value, var->value_len);
if (target == NULL) return -1;
left = atoi(target);
right = atoi(str.value);
left = atoll(target);
right = atoll(str.value);

if (left != right) {
/* No match. */
return 0;
}
else {
*error_msg = apr_psprintf(msr->mp, "Operator EQ matched %d at %s.", right, var->name);
*error_msg = apr_psprintf(msr->mp, "Operator EQ matched %s at %s.", str.value, var->name);
/* Match. */
return 1;
}
Expand All @@ -4456,7 +4456,7 @@ static int msre_op_gt_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char **error_msg)
{
msc_string str;
int left, right;
long long left, right;
char *target = NULL;

if ((var->value == NULL)||(rule->op_param == NULL)) {
Expand All @@ -4479,15 +4479,15 @@ static int msre_op_gt_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,

target = apr_pstrmemdup(msr->mp, var->value, var->value_len);
if (target == NULL) return -1;
left = atoi(target);
right = atoi(str.value);
left = atoll(target);
right = atoll(str.value);

if (left <= right) {
/* No match. */
return 0;
}
else {
*error_msg = apr_psprintf(msr->mp, "Operator GT matched %d at %s.", right, var->name);
*error_msg = apr_psprintf(msr->mp, "Operator GT matched %s at %s.", str.value, var->name);
/* Match. */
return 1;
}
Expand All @@ -4499,7 +4499,7 @@ static int msre_op_lt_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char **error_msg)
{
msc_string str;
int left, right;
long long left, right;
char *target = NULL;

if ((var->value == NULL)||(rule->op_param == NULL)) {
Expand All @@ -4522,15 +4522,15 @@ static int msre_op_lt_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,

target = apr_pstrmemdup(msr->mp, var->value, var->value_len);
if (target == NULL) return -1;
left = atoi(target);
right = atoi(str.value);
left = atoll(target);
right = atoll(str.value);

if (left >= right) {
/* No match. */
return 0;
}
else {
*error_msg = apr_psprintf(msr->mp, "Operator LT matched %d at %s.", right, var->name);
*error_msg = apr_psprintf(msr->mp, "Operator LT matched %s at %s.", str.value, var->name);
/* Match. */
return 1;
}
Expand All @@ -4542,7 +4542,7 @@ static int msre_op_ge_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char **error_msg)
{
msc_string str;
int left, right;
long long left, right;
char *target = NULL;

if ((var->value == NULL)||(rule->op_param == NULL)) {
Expand All @@ -4565,15 +4565,15 @@ static int msre_op_ge_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,

target = apr_pstrmemdup(msr->mp, var->value, var->value_len);
if (target == NULL) return -1;
left = atoi(target);
right = atoi(str.value);
left = atoll(target);
right = atoll(str.value);

if (left < right) {
/* No match. */
return 0;
}
else {
*error_msg = apr_psprintf(msr->mp, "Operator GE matched %d at %s.", right, var->name);
*error_msg = apr_psprintf(msr->mp, "Operator GE matched %s at %s.", str.value, var->name);
/* Match. */
return 1;
}
Expand All @@ -4585,7 +4585,7 @@ static int msre_op_le_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char **error_msg)
{
msc_string str;
int left, right;
long long left, right;
char *target = NULL;

if ((var->value == NULL)||(rule->op_param == NULL)) {
Expand All @@ -4608,15 +4608,15 @@ static int msre_op_le_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,

target = apr_pstrmemdup(msr->mp, var->value, var->value_len);
if (target == NULL) return -1;
left = atoi(target);
right = atoi(str.value);
left = atoll(target);
right = atoll(str.value);

if (left > right) {
/* No match. */
return 0;
}
else {
*error_msg = apr_psprintf(msr->mp, "Operator LE matched %d at %s.", right, var->name);
*error_msg = apr_psprintf(msr->mp, "Operator LE matched %s at %s.", str.value, var->name);
/* Match. */
return 1;
}
Expand Down