Added some null pointer checks.

Added a design doc.
This commit is contained in:
Marc Stern 2024-04-04 15:45:55 +02:00
parent 91da5872c1
commit 538ffa6baa
5 changed files with 43 additions and 15 deletions

View File

@ -867,10 +867,7 @@ static int hook_request_early(request_rec *r) {
* create the initial configuration.
*/
msr = create_tx_context(r);
if (msr == NULL) {
msr_log(msr, 9, "Failed to create context after request failure.");
return DECLINED;
}
if (msr == NULL) return DECLINED;
if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Context created after request failure.");
}
@ -1165,11 +1162,7 @@ static void hook_error_log(const char *file, int line, int level, apr_status_t s
#else
msr = create_tx_context((request_rec*)r);
#endif
if (msr == NULL) {
msr_log(msr, 9, "Failed to create context after request failure.");
return;
}
if (msr->txcfg->debuglog_level >= 9) {
if (msr && msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Context created after request failure.");
}
}

View File

@ -87,6 +87,7 @@ int json_add_argument(modsec_rec *msr, const char *value, unsigned length)
static int yajl_map_key(void *ctx, const unsigned char *key, size_t length)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
unsigned char *safe_key = (unsigned char *) NULL;
/**
@ -118,6 +119,7 @@ static int yajl_map_key(void *ctx, const unsigned char *key, size_t length)
static int yajl_null(void *ctx)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
return json_add_argument(msr, "", 0);
}
@ -128,6 +130,7 @@ static int yajl_null(void *ctx)
static int yajl_boolean(void *ctx, int value)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
if (value) {
return json_add_argument(msr, "true", strlen("true"));
@ -143,6 +146,7 @@ static int yajl_boolean(void *ctx, int value)
static int yajl_string(void *ctx, const unsigned char *value, size_t length)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
return json_add_argument(msr, value, length);
}
@ -155,12 +159,14 @@ static int yajl_string(void *ctx, const unsigned char *value, size_t length)
static int yajl_number(void *ctx, const char *value, size_t length)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
return json_add_argument(msr, value, length);
}
static int yajl_start_array(void *ctx) {
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
if (!msr->json->current_key && !msr->json->prefix) {
msr->json->prefix = apr_pstrdup(msr->mp, "array");
@ -190,6 +196,7 @@ static int yajl_start_array(void *ctx) {
static int yajl_end_array(void *ctx) {
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
unsigned char *separator = (unsigned char *) NULL;
/**
@ -226,6 +233,7 @@ static int yajl_end_array(void *ctx) {
static int yajl_start_map(void *ctx)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
/**
* If we do not have a current_key, this is a top-level hash, so we do not
@ -264,6 +272,7 @@ static int yajl_start_map(void *ctx)
static int yajl_end_map(void *ctx)
{
modsec_rec *msr = (modsec_rec *) ctx;
assert(msr != NULL);
unsigned char *separator = (unsigned char *) NULL;
/**

View File

@ -24,9 +24,6 @@ void validate_quotes(modsec_rec *msr, char *data, char quote) {
assert(msr != NULL);
int i, len;
if(msr == NULL)
return;
if(msr->mpd == NULL)
return;

View File

@ -215,11 +215,9 @@ char *msre_ruleset_phase_rule_update_target_matching_exception(modsec_rec *msr,
rules = (msre_rule **)phase_arr->elts;
for (i = 0; i < phase_arr->nelts; i++) {
msre_rule *rule = (msre_rule *)rules[i];
if (mode == 0) { /* Looking for next rule. */
if (msre_ruleset_rule_matches_exception(rule, re)) {
err = update_rule_target_ex(NULL, ruleset, rule, p2, p3);
err = update_rule_target_ex(msr, ruleset, rule, p2, p3);
if (err) return err;
if (rule->actionset->is_chained) mode = 2; /* Match all rules in this chain. */
} else {
@ -3141,6 +3139,8 @@ static apr_status_t msre_rule_process_normal(msre_rule *rule, modsec_rec *msr) {
/* Perform transformations. */
tarr = apr_table_elts(normtab);
/* if no transformation, multi_match makes no sense and breaks the logic */
if (tarr->nelts == 0) multi_match = 0;
/* Execute transformations in a loop. */

29
design.md Normal file
View File

@ -0,0 +1,29 @@
Design notes for source code
==
This file give some explanations and guidelines regarding ModSecurity v2 source code.
The goal is to discuss topics that are not related to a specific location in the code, so that cannot be best explained by comments.
The goal is not to replace comments where it is probably better.
It's quite short for the moment, but the goal is to extend it from time to time.
## Null pointer check
The default behaviour is to check for null pointer dereference everywhere it may be needed.
In case a pointer cannot be null, it has to be explained with a comment at the beginning of the function of when dereferencing the pointer.
On top of that, an explicit check should be done when compiling in debug mode with the following code:
```
assert(mypointer);
```
In case a pointer that cannot be null is used at several locations (say more than 3 times),
the explanation could be given globally in this file.
### Pointers never null
The following pointers can never be null:
#### msr
msr is assigned at the following places:
- mod_security2.c (14 x): initialization
In all the above calls, and all calling functions, it immediately returns (with an error code) in case msr is null, up to a place where no mod_security2 processing at all occurs.
In subsequent calls, there's thus no possibility to have msr null.
- apache2_io.c (2 x): assign a previously initialized msr
- msc_json (9 x): assign a previously initialized msr
- msc_lua.c (4 x): assign a previously initialized msr