Fix operator @validateByteRange working with bytes > 127

[asterite3]

ValidateByteRange::evaluate() acquires bytes it validates by indexing std::string:

bool ValidateByteRange::evaluate(Transaction *transaction, Rule *rule,
    const std::string &input, std::shared_ptr<RuleMessage> ruleMessage) {
    bool ret = true;

    size_t count = 0;
    for (int i = 0; i < input.length(); i++) {
        int x = input.at(i);
        if (!(table[x >> 3] & (1 << (x & 0x7)))) {
            ...
            count++;
        }
    }

    ret = (count != 0);
    // ...
    return ret;
}

The problem is that indexing a string gives a char, which is signed, so bytes with codes > 127 will appear as negative integers, resulting in incorrect validation. This can be illustrated by this minimal example:

#include "modsecurity/modsecurity.h"
#include "modsecurity/rules.h"

static void logCb(void *data, const void *ruleMessagev) {
    std::cout << (char *) ruleMessagev << std::endl;
}

const char * req_uri = "/test?param=%D0%A2%D0%B0%D1%80%D0%B0%D0%B1%D0%B0%D0%BD";

int main() {
    modsecurity::ModSecurity *modsec = new modsecurity::ModSecurity();
    modsecurity::Rules *rules = new modsecurity::Rules();
    modsecurity::Transaction * modsecTransaction;

    modsec->setServerLogCb(logCb, modsecurity::TextLogProperty);

    rules->loadFromUri("test.conf");

    modsecTransaction = new modsecurity::Transaction(modsec, rules, NULL);
    modsecTransaction->processURI(req_uri, "GET", "1.1");
    modsecTransaction->processRequestHeaders();
    modsecTransaction->processRequestBody();

    delete modsecTransaction;
    delete rules;
    delete modsec;
    return 0;
}

And this test config:

SecRule REQUEST_URI "@validateByteRange 1-255" \
  id:920270

Even though this config allows any byte except NULL byte, URI in test will fail the check resulting in alert, because bytes \xd0, \xa2 etc are considered negative, hence < 1 and out of range. This breaks rules 920270 , 920271 etc from OWASP CRS (in fact, config including those rules with tx.paranoia_level=2 can be used as test.conf in the above example, the rules will be mistakenly triggered).

The proposed solution is to cast a byte from input string to unsigned char before further processing - bytes will then fall in range [0;255] and be validated as expected. In fact, the same thing is done in v2 (https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/re_operators.c#L4168):

int x = ((unsigned char *)var->value)[i];

[asterite3]

Also added a unit-test for this case: owasp-modsecurity/secrules-language-tests#3

[zimmerle]

Hi @asterite3,

Well noticed ;) Thanks! Patch is now merged. Sorry for the delay.