PortSwigger / burp-extensions-montoya-api

Burp Extensions Api
Other
125 stars 3 forks source link

HttpRequest path() method also includes parameters #27

Closed jerome-kleinen-kbc-be closed 1 year ago

jerome-kleinen-kbc-be commented 1 year ago

Hello,

I've been playing around with the montoya API and generally I think it's a huge improvement over the previous API, so thank you for that!

One small thing I noticed today is that the path() method of the HttpRequest interface also includes query parameters. Of course this is largely a matter of interpretation, but personally I would have expected to only get the actual path. The documentation https://portswigger.github.io/burp-extensions-montoya-api/javadoc/burp/api/montoya/http/message/requests/HttpRequest.html#path() states "the path and file in the request" which also hints in this direction.

Furthermore I couldn't find any utility method which would be useful for easily stripping the query parameters from the path (as it's currently returned).

Thanks,

Jerome

SeanBurnsUK commented 1 year ago

Thanks for the feedback.

We are discussing adding some helper methods to get the query and the pathWithoutQuery.

The path in HttpRequest represents the data between the request method and the http version on the request line for HTTP/1 and it represents the contents of the :path pseudo header for HTTP/2.

floyd-fuh commented 1 year ago

@SeanBurnsUK: pathWithoutQuery is called path in Java and path is called file in Java. Here: https://docs.oracle.com/javase/7/docs/api/java/net/URL.html#getFile() . Why don't you stick to Java naming conventions/RFCs for things that are clearly defined?

Also applies to the old API. I don't get it what Burp's logic is supposed to do.

It gets even worse, because your explanation above isn't true either:

RFC 3986 (URI): The path is terminated by the first question mark ("?") Me: Hey Java, parse URL("http://a.com/f.php?a=b?c=d").path Java: Result is /f.php Me: Hey Burp extension API, parse helpers.analyzeRequest([...] request with "http://a.com/f.php?a=b?c=d").url.path Burp: YOLO, result is /f.php?a=b

This looks pretty bad to me. Because that's not even the right answer if you want to do it wrong (that would be /f.php?a=b?c=d then).

Hannah-PortSwigger commented 1 year ago

We realize this behavior is confusing, and we will add further helper methods in the future.

HTTP/2 includes the query parameters in the :path pseudoheader, so we opted to give users the most amount of information, as you can always parse the given string to retrieve specific data.

floyd-fuh commented 1 year ago

Ok, I can see that logic. But why is /f.php?a=b returned for the URL /f.php?a=b?c=d? Is that considered a bug or as intended?

Hannah-PortSwigger commented 1 year ago

It looks like you're using the legacy Extender API rather than the Montoya API.

We've had a look into this, and the returned URL of an analyzed request includes both ?a=b and ?c=d.

When calling Java's getPath() method on the returned java.net.URL object, ?c=d does get stripped off. This is not to do with Burp's code, but rather, this is due to the behavior of Java.