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 all commits
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
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;

Comment on lines +303 to 305
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

In sanitize_request_line, j is now size_t but arg_min/arg_max remain int. A few lines later arg_min = j; assigns the result of strlen() (size_t) into an int, which can truncate on 64-bit and may reintroduce signed/unsigned warnings. Consider making arg_min/arg_max (and related computations) size_t (or removing arg_min if it’s unused outside the loop).

Copilot uses AI. Check for mistakes.
/* 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);
int off = (int)strlen(mparm->value) - arg_max;
int pos = (mparm->pad_1-1);
if(off > pos) {
Comment on lines 336 to 339
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

off is computed as (int)strlen(mparm->value) - arg_max. Casting strlen() down to int can truncate for long values and can also mask underflow/overflow issues in the subtraction. Prefer doing this arithmetic in size_t (or ptrdiff_t if negatives are meaningful) and compare using consistent unsigned types.

Copilot uses AI. Check for mistakes.
*pat = '*';
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;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

In sec_audit_logger_json, j is now size_t but arg_min/arg_max are still int and later receive j (derived from strlen). This can truncate on 64-bit and may defeat the goal of eliminating warnings. Use size_t for the related variables (or add an explicit, checked cast if these are intentionally bounded).

Suggested change
int arg_min, arg_max, sanitize_matched;
size_t arg_min, arg_max;
int sanitize_matched;

Copilot uses AI. Check for mistakes.
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;
int off = (int)strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
Comment on lines +832 to 834
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

Same as earlier in this file: int off = (int)strlen(mparm->value) - arg_max; performs potentially truncating arithmetic on strlen() results. Keeping off/arg_max/pos in a consistent unsigned (or signed) width avoids truncation and signed/unsigned comparison problems.

Suggested change
int off = (int)strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
size_t off = strlen(mparm->value) - (size_t)arg_max;
int pos = mparm->pad_1-1;
if((pos < 0) || (off > (size_t)pos)) {

Copilot uses AI. Check for mistakes.
*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;
int off = (int)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;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

In sec_audit_logger_native, j is now size_t but arg_min/arg_max remain int and are assigned from strlen()-derived values. This can truncate and may still trigger signed/unsigned warnings on some toolchains. Consider updating these related counters/lengths to size_t for consistency.

Suggested change
int arg_min, arg_max, sanitize_matched;
size_t arg_min, arg_max;
int sanitize_matched;

Copilot uses AI. Check for mistakes.
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;
int off = (int)strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
Comment on lines +1689 to 1691
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

Same truncation risk here: int off = (int)strlen(mparm->value) - arg_max;. Since strlen() returns size_t, doing the math in int can overflow/truncate and makes the code less portable. Prefer using size_t (or ptrdiff_t) for off and adjust comparisons accordingly.

Suggested change
int off = (int)strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
size_t off = strlen(mparm->value) - (size_t)arg_max;
int pos = mparm->pad_1-1;
if ((pos < 0) || (off > (size_t)pos)) {

Copilot uses AI. Check for mistakes.
*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;
int off = (int)strlen(mparm->value) - arg_max;
int pos = mparm->pad_1-1;
if(off > pos) {
*pat = '*';
Expand Down
9 changes: 5 additions & 4 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;
size_t i;
size_t len;

if(msr->mpd == NULL)
return;
Expand All @@ -42,7 +43,7 @@ void validate_quotes(modsec_rec *msr, char *data, char quote) {

if(data[i] == '\'') {
if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Multipart: Invalid quoting detected: %s length %d bytes",
msr_log(msr, 9, "Multipart: Invalid quoting detected: %s length %zu bytes",
log_escape_nq(msr->mp, data), len);
}
msr->mpd->flag_invalid_quoting = 1;
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
8 changes: 4 additions & 4 deletions apache2/msc_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,12 +274,12 @@ 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;
size_t offset = 0;
char converted = 0;
int i, x;
size_t i, x;
unsigned char bin = 0, esc = 0, bin_offset = 0;
unsigned char c = 0;
unsigned char bin_parm[3] = { 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
23 changes: 13 additions & 10 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 @@ -1361,7 +1362,7 @@ static int msre_op_pmFromFile_param_init(msre_rule *rule, char **error_msg) {
char *end = NULL;
const char *rulefile_path;
char *processed = NULL;
unsigned short int op_len;
size_t op_len;
apr_status_t rc;
apr_file_t *fd = NULL;
ACMP *p;
Expand Down Expand Up @@ -1702,7 +1703,7 @@ static const char *gsb_reduce_char(apr_pool_t *pool, const char *domain) {
* \retval 1 On Match
* \retval 0 On No Match
*/
static int verify_gsb(gsb_db *gsb, modsec_rec *msr, const char *match, unsigned int match_length) {
static int verify_gsb(gsb_db *gsb, modsec_rec *msr, const char *match, size_t match_length) {
assert(gsb != NULL);
assert(msr != NULL);
assert(match != NULL);
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
Loading