Skip to content

fix: strlen related variables should be of type size_t #3503

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

Draft
fzipi wants to merge 2 commits into
base: v2/master
Choose a base branch
from
Draft

fix: strlen related variables should be of type size_t #3503

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86f4b3a
fix: use size_t type for len related vars
fzipi Feb 20, 2026
3f7bf11
fix: resolve signed/unsigned mismatches and incomplete size_t propaga…
fzipi Feb 20, 2026
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
2 changes: 1 addition & 1 deletion apache2/apache2_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ static void internal_log_ex(request_rec *r, directory_config *dcfg, modsec_rec *
/* Construct the message. */
apr_vsnprintf(str1, sizeof(str1), text, ap);
if (fixup) {
int len = strlen(str1);
size_t len = strlen(str1);

/* Strip line ending. */
if (len && str1[len - 1] == '\n') {
Expand Down
2 changes: 1 addition & 1 deletion apache2/msc_crypt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ int do_hash_method(modsec_rec *msr, char *link, int type) {
int hash_response_body_links(modsec_rec *msr) {
int lsize = 0, fsize = 0, lcount = 0, fcount = 0, i;
int isize = 0, icount = 0, frsize = 0, frcount = 0;
int bytes = 0;
size_t bytes = 0;
xmlXPathContextPtr xpathCtx = NULL;
xmlXPathObjectPtr xpathObj = NULL;
xmlChar *content_option = NULL;
Expand Down
19 changes: 11 additions & 8 deletions apache2/msc_logging.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,8 @@ static void sanitize_request_line(modsec_rec *msr) {
if (strcmp(arg->origin, "QUERY_STRING") == 0) {
char *pat = NULL;
char *p;
int j, arg_min, arg_max;
size_t j;
int arg_min, arg_max;

/* Go to the beginning of the parameter. */
p = qspos;
Expand Down Expand Up @@ -333,7 +334,7 @@ static void sanitize_request_line(modsec_rec *msr) {
arg_max = 1;
while((*pat != '\0')&&(j--)) {
if(arg_max > mparm->pad_2) {
int off = (strlen(mparm->value) - arg_max);
size_t off = (strlen(mparm->value) - arg_max);
int pos = (mparm->pad_1-1);
if(off > pos) {
*pat = '*';
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

off was changed to size_t, but it’s compared against pos which can be negative when mparm->pad_1 is 0 (or -1). The signed/unsigned comparison changes behavior (negative pos becomes a huge size_t), which can prevent intended masking and potentially leak sensitive data. Keep off as a signed type (e.g., ptrdiff_t/ssize_t/int with range checks) or clamp/handle negative pos explicitly before comparing.

Copilot uses AI. Check for mistakes.
Expand Down Expand Up @@ -668,7 +669,8 @@ void sec_audit_logger_json(modsec_rec *msr) {
int wrote_response_body = 0;
char *entry_filename, *entry_basename;
apr_status_t rc;
int i, limit, k, sanitized_partial, j;
int i, limit, k, sanitized_partial;
size_t j;
char *buf = NULL, *pat = NULL;
msc_parm *mparm = NULL;
int arg_min, arg_max, sanitize_matched;
Expand Down Expand Up @@ -827,7 +829,7 @@ void sec_audit_logger_json(modsec_rec *msr) {
arg_max = 1;
while((*pat != '\0')&&(j--)) {
if(arg_max > mparm->pad_2) {
int off = strlen(mparm->value) - arg_max;
size_t off = strlen(mparm->value) - arg_max;
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

Same signed/unsigned masking issue here: off is size_t but pos is derived from mparm->pad_1-1 and can be negative. This changes the masking condition and can result in headers not being partially sanitized as configured. Use a signed off (or handle negative pos) to preserve the intended behavior.

Suggested change
size_t off = strlen(mparm->value) - arg_max;
int off = (int)strlen(mparm->value) - arg_max;

Copilot uses AI. Check for mistakes.
int pos = mparm->pad_1-1;
if(off > pos) {
*pat = '*';
Expand Down Expand Up @@ -1084,7 +1086,7 @@ void sec_audit_logger_json(modsec_rec *msr) {
arg_max = 1;
while((*pat != '\0')&&(j--)) {
if(arg_max > mparm->pad_2) {
int off = strlen(mparm->value) - arg_max;
size_t off = strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
*pat = '*';
Expand Down Expand Up @@ -1547,7 +1549,8 @@ void sec_audit_logger_native(modsec_rec *msr) {
int wrote_response_body = 0;
char *entry_filename, *entry_basename;
apr_status_t rc;
int i, limit, k, sanitized_partial, j;
int i, limit, k, sanitized_partial;
size_t j;
char *buf = NULL, *pat = NULL;
msc_parm *mparm = NULL;
int arg_min, arg_max, sanitize_matched;
Expand Down Expand Up @@ -1683,7 +1686,7 @@ void sec_audit_logger_native(modsec_rec *msr) {
arg_max = 1;
while((*pat != '\0')&&(j--)) {
if(arg_max > mparm->pad_2) {
int off = strlen(mparm->value) - arg_max;
size_t off = strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
*pat = '*';
Expand Down Expand Up @@ -1931,7 +1934,7 @@ void sec_audit_logger_native(modsec_rec *msr) {
arg_max = 1;
while((*pat != '\0')&&(j--)) {
if(arg_max > mparm->pad_2) {
int off = strlen(mparm->value) - arg_max;
size_t off = strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
*pat = '*';
Expand Down
7 changes: 4 additions & 3 deletions apache2/msc_multipart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

void validate_quotes(modsec_rec *msr, char *data, char quote) {
assert(msr != NULL);
int i, len;
int i;
size_t len;

if(msr->mpd == NULL)
return;
Expand Down Expand Up @@ -846,7 +847,7 @@ int multipart_init(modsec_rec *msr, char **error_msg) {
char *p = NULL;
char *b = NULL;
int seen_semicolon = 0;
int len = 0;
size_t len = 0;

/* Check for extra characters before the boundary. */
for (p = (char *)(msr->request_content_type + 19); p < msr->mpd->boundary; p++) {
Expand Down Expand Up @@ -1485,7 +1486,7 @@ int multipart_get_arguments(modsec_rec *msr, char *origin, apr_table_t *argument
char *multipart_reconstruct_urlencoded_body_sanitize(modsec_rec *msr) {
multipart_part **parts;
char *body;
unsigned int body_len;
size_t body_len;
int i;

if (msr->mpd == NULL) return NULL;
Expand Down
6 changes: 3 additions & 3 deletions apache2/msc_status_engine.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@
// Bese32 encode, based on:
// https://code.google.com/p/google-authenticator/source/browse/libpam/base32.c
int DSOLOCAL msc_status_engine_base32_encode(char *encoded,
const char *data, int len) {
const char *data, size_t len) {
int buffer;
int count = 0;
char *result = encoded;
int length = strlen(data);
size_t length = strlen(data);

buffer = data[0];

Expand Down Expand Up @@ -97,7 +97,7 @@ int DSOLOCAL msc_status_engine_base32_encode(char *encoded,
}

int DSOLOCAL msc_status_engine_fill_with_dots(char *encoded_with_dots,
const char *data, int len, int space)
const char *data, size_t len, int space)
{
int i;
int count = 0;
Expand Down
4 changes: 2 additions & 2 deletions apache2/msc_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ unsigned char is_netmask_v6(char *ip_strv6) {
*
* \retval string On Success
*/
char *parse_pm_content(const char *op_parm, unsigned short int op_len, msre_rule *rule, char **error_msg) {
char *parse_pm_content(const char *op_parm, size_t op_len, msre_rule *rule, char **error_msg) {
char *parm = NULL;
char *content = NULL;
unsigned short int offset = 0;
Expand Down Expand Up @@ -708,7 +708,7 @@ char *file_basename(apr_pool_t *mp, const char *filename) {

char *m_strcasestr(const char *haystack, const char *needle) {
char aux, lower_aux;
int length;
size_t length;

if ((aux = *needle++) != 0) {
aux = (char)tolower((unsigned char)aux);
Expand Down
2 changes: 1 addition & 1 deletion apache2/msc_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ int DSOLOCAL parse_boolean(const char *input);

char DSOLOCAL *remove_quotes(apr_pool_t *mptmp, const char *input, int input_len);

char DSOLOCAL *parse_pm_content(const char *op_parm, unsigned short int op_len, msre_rule *rule, char **error_msg);
char DSOLOCAL *parse_pm_content(const char *op_parm, size_t op_len, msre_rule *rule, char **error_msg);

char DSOLOCAL *remove_escape(apr_pool_t *mptmp, const char *input, int input_len);

Expand Down
19 changes: 11 additions & 8 deletions apache2/re_operators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ static int msre_op_rsub_param_init(msre_rule *rule, char **error_msg) {
char *data = NULL;
char delim;
int ignore_case = 0;
unsigned short int op_len = 0;
size_t op_len = 0;

*error_msg = NULL;

Expand Down Expand Up @@ -529,7 +529,8 @@ static int msre_op_rsub_execute(modsec_rec *msr, msre_rule *rule, msre_var *var,
char *data_out = NULL;
unsigned int size = 0;
unsigned int maxsize=0;
int output_body = 0, input_body = 0, sl;
int output_body = 0, input_body = 0;
size_t sl;
#if AP_SERVER_MAJORVERSION_NUMBER > 1 && AP_SERVER_MINORVERSION_NUMBER > 0
ap_regmatch_t pmatch[AP_MAX_REG_MATCH];
#else
Expand Down Expand Up @@ -1315,7 +1316,7 @@ static int msre_op_pm_param_init(msre_rule *rule, char **error_msg) {
ACMP *p;
const char *phrase;
const char *next;
unsigned short int op_len;
size_t op_len;

if ((rule->op_param == NULL)||(strlen(rule->op_param) == 0)) {
*error_msg = apr_psprintf(rule->ruleset->mp, "Missing parameter for operator 'pm'.");
Expand Down Expand Up @@ -1794,15 +1795,16 @@ static int msre_op_gsbLookup_execute(modsec_rec *msr, msre_rule *rule, msre_var
int options = 0;
gsb_db *gsb = msr->txcfg->gsb;
const char *match = NULL;
unsigned int match_length;
unsigned int canon_length;
size_t match_length;
size_t canon_length;
int rv, i, ret, count_slash;
unsigned int j = 0;
unsigned int size = var->value_len;
char *base = NULL, *domain = NULL, *savedptr = NULL;
char *str = NULL, *canon = NULL, *dot = NULL;
char *data = NULL, *ptr = NULL, *url = NULL;
int capture, domain_len;
int capture;
size_t domain_len;
int d_pos = -1;
int s_pos = -1;

Expand Down Expand Up @@ -2672,7 +2674,7 @@ static int msre_op_strmatch_param_init(msre_rule *rule, char **error_msg) {
const apr_strmatch_pattern *compiled_pattern;
char *processed = NULL;
const char *pattern = rule->op_param;
unsigned short int op_len;
size_t op_len;

*error_msg = NULL;

Expand Down Expand Up @@ -4166,7 +4168,8 @@ static int msre_op_fuzzy_hash_init(msre_rule *rule, char **error_msg)
struct fuzzy_hash_chunk *chunk, *t;
FILE *fp;
char *file;
int param_len,threshold;
size_t param_len;
int threshold;
char line[1024];

char *data = NULL;
Expand Down
2 changes: 1 addition & 1 deletion apache2/re_variables.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2484,7 +2484,7 @@ static int var_full_request_generate(modsec_rec *msr, msre_var *var,
char *full_request = NULL;
int full_request_length = 0;
int headers_length = 0;
int request_line_length = 0;
size_t request_line_length = 0;

arr = apr_table_elts(msr->request_headers);
headers_length = msc_headers_to_buffer(arr, NULL, 0);
Expand Down
Loading