Skip to content
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

Whitelist form methods in FormRequest.from_response #3777

csalazar opened this issue May 14, 2019 · 3 comments

Whitelist form methods in FormRequest.from_response #3777

csalazar opened this issue May 14, 2019 · 3 comments


Copy link

@csalazar csalazar commented May 14, 2019

Hi team, according to this article, there are 3 methods that are accepted in form's method attribute. I can't remember about other methods, but I'd agree to consider the rest of REST verbs if that's common. Anyway, I think it should be a good idea to whitelist the accepted methods to avoid scenarios like this vulnerability exploitation that took advantage of form's method.

This issue affects FormRequest.from_response and the affected line is:

method = kwargs.pop('method', form.method)

I want to know if there some reason behind this behavior, otherwise I could send a pull request.

Copy link

@dangra dangra commented May 16, 2019

Hi @csalazar, well done on the telnet security issue article you published, I loved the attack vector explanation.

You demonstrated clearly how Form's method can be exploited on code injection, so no doubts we would like to restrict the set of valid methods. My suggestion would be to mimic what major browsers are accepting.

Copy link

@subhamkrai subhamkrai commented Jun 10, 2019

Hi I would like to work on this issue. I am newbie please describe me.

Copy link

@Gallaecio Gallaecio commented Jun 14, 2019

@subhamkrai Thank you very much, however there is already a solution being worked at #3794

@kmike kmike closed this in #3794 Jul 2, 2019
kmike added a commit that referenced this issue Jul 2, 2019

[MRG+1] Fix form methods in FormRequest.from_response (#3777)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants