Skip to content

Possible representation bug in uploads part of JSON audit log format #1173

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.

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

Closed
dune73 opened this issue Jun 28, 2016 · 8 comments
Closed

Possible representation bug in uploads part of JSON audit log format #1173

dune73 opened this issue Jun 28, 2016 · 8 comments
Assignees
Labels
bug It is a confirmed bug TBF by libmodsec

Comments

@dune73
Copy link
Member

dune73 commented Jun 28, 2016

I have the following entry in the JSON audit log format:

...
    "uploads": {
        "info": [
            "file_size",
            48458,
            "file_name",
            "example.png",
            "content_type",
            "<Unknown Content-Type>"
        ],
        "total": 48458
    }
...

Within the info tag, there is a list of items and not a list of key-value pairs as would be expected. It might have to do with the way the data is represented within ModSecurity (-> corresponding native audit log format part "J" is a CSV), but this seems to be a bug to me.

@zimmerle
Copy link
Contributor

Linking this issue with #1174, as they seems to be on the same piece of code.

@zimmerle zimmerle self-assigned this Jun 29, 2016
@zimmerle zimmerle added bug It is a confirmed bug TBF by libmodsec labels Jun 29, 2016
@p0pr0ck5
Copy link
Contributor

p0pr0ck5 commented Jun 29, 2016

Not the same issue as #1174, but looks like an easy fix. msc_logging.c:1363 opens this with yajl_gen_array_open but is missing a separate yajl_gen_map_open for each upload. Whups. I'll have a PR up for this within a day or two.

@zimmerle
Copy link
Contributor

Thanks @p0pr0ck5 ;)

@dune73
Copy link
Member Author

dune73 commented Jun 29, 2016

Thank you guys.

p0pr0ck5 added a commit to p0pr0ck5/ModSecurity that referenced this issue Jun 29, 2016
Each uploaded file is a separate yajl array, but we forgot to open
the a map for the proper k/v pairs.

This fixes issue owasp-modsecurity#1173.
@p0pr0ck5
Copy link
Contributor

@dune73 can you try the fix in p0pr0ck5@e54b150? I won't have time to test for a bit but this should resolve the issue, small oversight on my part. We don't log file upload data so I wasn't as thorough as I should have been.

@p0pr0ck5
Copy link
Contributor

^ Once this is tested I'll submit the PR

@dune73
Copy link
Member Author

dune73 commented Jul 2, 2016

Looks good here:

    "uploads": {
        "info": [
            {
                "content_type": "<Unknown Content-Type>",
                "file_name": "tmp1",
                "file_size": 457
            }
        ],
        "total": 457
    }

Thank you for the quick fix.

zimmerle pushed a commit that referenced this issue Jul 11, 2016
Each uploaded file is a separate yajl array, but we forgot to open
the a map for the proper k/v pairs.

This fixes issue #1173.
@zimmerle
Copy link
Contributor

Pull request #1181 was merged! thanks!

mkubenka pushed a commit to mkubenka/ModSecurity that referenced this issue Aug 11, 2016
Each uploaded file is a separate yajl array, but we forgot to open
the a map for the proper k/v pairs.

This fixes issue owasp-modsecurity#1173.
mkubenka pushed a commit to mkubenka/ModSecurity that referenced this issue Aug 11, 2016
Each uploaded file is a separate yajl array, but we forgot to open
the a map for the proper k/v pairs.

This fixes issue owasp-modsecurity#1173.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It is a confirmed bug TBF by libmodsec
Projects
None yet
Development

No branches or pull requests

3 participants