zaproxy icon indicating copy to clipboard operation
zaproxy copied to clipboard

OpenAPI - Add support for application/xml

Open davewichers opened this issue 2 years ago • 22 comments

Describe the bug I have an openapi spec I'm loading into ZAP, that looks like this for 1 path:

/rest/sqli-00/BenchmarkTest00568/send:
  post:
    operationId: dopost_568
    requestBody:
      description: Form POST parameters in request body
      required: true
      content:
        application/xml:
          schema:
            type: object
            properties:
              name:
                type: string
              address:
                type: string
    responses:
      default:
        description: default response
        content:
          application/xml: {}

But ZAP is sending:

POST https://localhost:8443/benchmark/rest/sqli-00/BenchmarkTest00568/send HTTP/1.1 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:82.0) Gecko/20100101 Firefox/82.0 Pragma: no-cache Cache-Control: no-cache Content-Length: 57 Accept: application/xml Host: localhost:8443

{"name":"ZAP","address":"John Doe"}

As you can see, even though the request content type is: application/xml ZAP is sending application/json instead.

To Reproduce Load an API spec with application/xml as the request content type.

Expected behavior It should send XML formatted data in the request, not json.

Software versions

  • ZAP: 2.10.0
  • Add-on: OpenAPI plugin v20

davewichers avatar Aug 20 '21 22:08 davewichers

FYI. I just tried this again and its actually worse now. The content length of ANY Post, regardless of content type specified in the open api spec now has:

Content-Length: 0 Accept: */*

davewichers avatar Sep 06 '21 20:09 davewichers

Please, provide the definition that exhibits the issues.

thc202 avatar Sep 06 '21 21:09 thc202

I did. It is described in the issue. You need both the OpenAPI spec AND a target app to actually test against. I provided an example of a spec for a single test case. You can simply modify whatever test case you are testing against to specify:

  content:
        application/xml:

And then see that ZAP actually generates either a body like this: {"name":"ZAP","address":"John Doe"}

Or NO body at all, which is what it is generating now.

davewichers avatar Sep 06 '21 21:09 davewichers

Sorry, but an example path is not a definition. You mentioned several issues unrelated to application/xml, which is what brought me here.

thc202 avatar Sep 06 '21 22:09 thc202

This issue is about only the application/xml misbehavior, which you should be able to replicate by adding the example to pretty much any OpenAPI spec you test with, and then look at the request ZAP generates. The issue you are referring to regarding the failure to find the Server URL, I provided the details for that in that issue. The other issue I brought up at the same time is a red herring. Sorry about that.

davewichers avatar Sep 07 '21 00:09 davewichers

Sorry, but an example path is not a definition. You mentioned several issues unrelated to application/xml, which is what brought me here.

If you need an OpenAPI spec to test with, use this: https://github.com/OWASP/Benchmark/blob/master/data/openapi.yaml - You can also test this against the Benchmark itself, so you have an app to test it against. While the current Benchmark only accepts text/html, you can edit one of the endpoints to instead specify:

content:
        application/xml:

And then you should see that instead of ZAP sending whatever parameters are specified as a block of XML, it sends them as a block of JSON instead. Once this is fixed, it should send XML instead. While Benchmark 1.2 won't accept the XML, I can test against the test app I'm developing that does, which isn't publicly available yet.

If you actually have access to a web app that accepts XML and has an OpenAPI spec, you can use that to test with of course.

davewichers avatar Sep 08 '21 18:09 davewichers

Could you provide the XML for the given example? I don't know what's the expected root element.

thc202 avatar Sep 26 '21 09:09 thc202

Ref OAI/OpenAPI-Specification#2097

thc202 avatar Sep 26 '21 10:09 thc202

The add-on will no longer generate JSON content for XML, though it still does not support XML (per previous referenced issue).

thc202 avatar Sep 28 '21 15:09 thc202

OK. That's 'better', but do you plan to implement the XML support for it?

And you asked about the beginning of the openapi file. It is:

info:
  title: OWASP Benchmark Test Application
  description: 'This is the Open API spec for the OWASP Benchmark AppSec Tools Test application for Java.  You
    can find more info at: [https://github.com/OWASP/Benchmark](https://github.com/OWASP/Benchmark)'
  contact:
    email: [email protected]
  license:
    name: GNU GPL 2.0
    url: https://choosealicense.com/licenses/gpl-2.0/
  version: "1.2"
servers:
- url: https://localhost:8443/benchmark
paths:

And then after that are things like: /sqli-00/BenchmarkTest00001 ...

to eventually:

/rest/sqli-00/BenchmarkTest00568/send:

davewichers avatar Sep 28 '21 23:09 davewichers

Yes, as long as it's possible to generate the XML properly (which isn't at the moment, the OpenAPI specification does not allow to define the root elements of the request bodies).

I was actually asking for the expected XML (though just interested in the root element) not the beginning of the OpenAPI definition.

thc202 avatar Sep 29 '21 08:09 thc202

Could you provide the XML for the given example? I don't know what's the expected root element.

I thought it was something like this:

<name>David Wichers</name><address>some address</address>

But it is actually:

<person><name>David Wichers</name><address>some address</address></person>

So, I think I'm missing some specification of <person> as the required root element in my open API spec snippet I provided. I need to research how to specify that properly. Once I figure that out, I'll provide you an updated OpenAPI spec snippet. Or, if you figure out how it is supposed to be, you can let me know. I'm on vacation currently so not sure when I'll have time to research that. I might be able to do so this afternoon, but not sure.

I'm crafting this OpenAPI spec by hand with custom code, so I'm probably doing something wrong.

davewichers avatar Sep 29 '21 12:09 davewichers

At the moment it's not possible to define that directly in the request body, you'd have to declare the objects in the components and reference those: https://swagger.io/specification/#xmlObject (Though the parser loses the name of the schema object when the references are resolved, but that would be another problem.)

This one has more/better examples: https://swagger.io/docs/specification/data-models/representing-xml/

thc202 avatar Sep 29 '21 13:09 thc202

Thanks for researching! Based on these references, it looks like I'll need to make two changes:

  1. Add this components block:
components:
  schemas:
    person:
      type: object
      properties:
        name:
          type: string
        address:
          type: string
  1. And then update each path with content type: application/xml that includes a <person> to say:
  /rest/sqli-00/BenchmarkTest00568/send:
    post:
      operationId: dopost_568
      requestBody:
        description: Form POST parameters in request body
        required: true
        content:
          application/xml:
            schema:
              $ref: '#/components/schemas/person'
      responses:
  ... same as before ...

Does that look right to you?

Also, does it matter if the 'components:' block is before or after the 'paths:'? Seems like the location of that block shouldn't matter. Not sure if something has to be declared first, before it can be referenced.

davewichers avatar Oct 18 '21 18:10 davewichers

Yes, that looks right. The order of the fields does not matter.

I'll have to check the parser library to know (if and) how to keep the names of the references to create the root element.

thc202 avatar Oct 22 '21 16:10 thc202

@thc202 - It's been 3+ months. Can you look into this? I'd really like to see this feature implemented as I plan to release an update to Benchmark that will require this to scan it. Thx.

davewichers avatar Feb 15 '22 15:02 davewichers

He is very busy working on essential networking changes. As I am concerned this issue is a lower priority :)

psiinon avatar Feb 15 '22 16:02 psiinon

We are also highly interested in this feature. One question: Does it work (e.g. sending XML bodies etc) for SOAP Request (which is usually XML based)? Because then it might not be such a big change because it does exists for a different type of API...?

Lonzak avatar Aug 29 '23 11:08 Lonzak

Hello, is there any update on the timeline of this feature ? We are also highly interested in it.

dimitriospapadimas avatar Mar 28 '24 09:03 dimitriospapadimas

Not that I'm aware of. See https://www.zaproxy.org/faq/how-do-i-get-a-specific-feature-implemented-in-zap/ Also note that you can now sponsor developments: https://www.zaproxy.org/support/#sponsored-developments - we should update the above FAQ...

psiinon avatar Mar 28 '24 09:03 psiinon