Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 20 Mar 2013 18:04:09 +0000 (GMT)
From: Costin Enache <e_costin@...oo.com>
To: "john-dev@...ts.openwall.com" <john-dev@...ts.openwall.com>
Subject: Re: err in rules processor and dirty fix [rules.c]

Hi,
 
The 4x change solves the hex encoding, but I don't think
it's enough. The rules_reject function is used for both expanded and
unprocessed data. For expanded rules, the size is ok, and with the 4x increase,
it fits also hex values. However, the same RULE_BUFFER_SIZE must fit (fortunately,
only to check for invalid rules) the unprocessed ones, which can be very long, mostly
because of character ranges.
 
Take my case: I need to prepend and append
machine-generated characters, and I am too lazy to check what is
"common" and what is "special" character, so the easiest way
is to encode all of them to hex representation. It is very easy to reach the
buffer limit. As a temporary solution, I have increased the RULE_BUFFER_SIZE
value in params.h, to 0x5000. This is overkill for already processed/ expanded
rules. We could have two "rules_reject" functions, one of expanded
and one for unprocessed rules ... not sure this is ok.
 
I'd avoid patching for now, and it would be better if the
original author of the rules parsing code could have a look into this, and see
what needs to be changed (otherwise we might overlook some implication of the
changes, get funny errors later, or waste memory on already processed rules).
 
Cheers,
Costin



________________________________
 From: magnum <john.magnum@...hmail.com>
To: john-dev@...ts.openwall.com 
Sent: Wednesday, 20 March 2013, 0:31
Subject: Re: [john-dev] err in rules processor and dirty fix [rules.c]
 
On 15 Mar, 2013, at 17:51 , Costin Enache <e_costin@...oo.com> wrote:
> Ugly thing in the rule processor, when the rules are using \xHH representation and the rule line size exceeds 0x100; this is only my particular case, it can generate errors in many other ways.
>  
> The (params.h) RULE_BUFFER_SIZE is used for the rule pre-processor output, as defined in rpp.h:
> /* Current rule after preprocessing */
>         char output[RULE_BUFFER_SIZE];
>  
> The same RULE_BUFFER_SIZE is used in rules.c in rules_reject:
> char *rules_reject(char *rule, int split, char *last, struct db_main *db)
> {
>         static char out_rule[RULE_BUFFER_SIZE];
>  
> The rules_reject function is called for unprocessed data, as it comes from the config file, for example to remote duplicate rules (rules_load_normalized_list).
>  
> The unprocessed data can easily be over 0x100, such as in my unfortunate case, when using \xHH for a long character list. In this case the remove duplicates function will only compare the first 0x100 bytes of the rules – dumping all the rules which begin with the same 0x100 bytes.
>  
> Quick and dumb fix, not sure that it is 100% ok and what else it breaks, but it solves my hex encoding problem:
>  
> --- rules.c.orig        2013-03-15 17:34:14.000000000 +0100
> +++ rules.c     2013-03-15 17:34:22.000000000 +0100
> @@ -701,7 +701,7 @@
>  char *rules_reject(char *rule, int split, char *last, struct db_main *db)
> {
> -       static char out_rule[RULE_BUFFER_SIZE];
> +       static char out_rule[4*RULE_BUFFER_SIZE];
>         while (RULE)
>         switch (LAST) {

Thank for reporting this! I'm pretty sure your fix is OK (perhaps similar fixes need to be in more places?). I will apply it to bleeding and do some testing. If all seems fine and unless Solar see a caveat, I will commit it to unstable.

magnum
Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.