Fix: FILES variable does not use multipart part name for key

[martinhsv]

No description provided.

[airween]

If I'm right, whit this patch the m_variableFiles and m_variableFilesNames transaction variables will have the same value (m->m_name from the body processor). These variables converted to FILES and FILES_NAMES here and here (and in the other function), so the FILES and FILES_NAMES will have the same value.

Or em I wrong?

(Note, there was a similar bug: the m_variableFiles and m_variableFilesNames both had the m_filename value, and fixed in #2016.)

[martinhsv]

Hi @airween ,

Thanks for the feedback. I'll explain in a little more detail and you can let me know if you still have questions or concerns.

The FILES_NAMES collection (i.e. m_variableFilesNames) has key-value pairs where both the key and the value are the same string, which means that with multipart part content including something ilke this ...

Content-Disposition: form-data; name="abcd"; filename="x00001.txt"

... you could test for the 'name' in either the key position of a rule or the value position of a rule, or both. For example:

SecRule FILES_NAMES:'/abc/' "@contains bcd" "id:1240, phase:2, deny,status:403"

This is similar to how other '_NAMES' collections work. For example, in ARGS_NAMES, the same string is both the key and the value in the key-value pair.

With this pull request, the FILES collection (i.e. m_variableFiles) entries will have a key that is the same as in FILES_NAMES, but the value will be the string from 'filename'. It enables a rule like this:

SecRule FILES:'/abc/' "@contains x000" "id:1240, phase:2, deny,status:403"

... which would not work in v3 prior to this pull request.

This is analogous to ARGS_NAMES vs. ARGS, where they both have the same key but the latter has a value that is different from the key.

Note also, that this change makes the functionality match that of v2.9.3.

Let me know if that clarifies things or if you still have questions or concerns. Thanks.

[airween]

Hi @martinhsv,

many thanks for details, now it's clear. I checked your patch with this test file (which - I think - demonstrates the advantage of the feature), it worked as well (without the patch the last case was failed).

Just my 2 cents: it would be good to add this behavior to the documentation.

[zimmerle]

On QA here - https://travis-ci.org/github/SpiderLabs/ModSecurity/builds/741866960

[zimmerle]

Merged! Thanks!