Skip to content
This repository has been archived by the owner on May 28, 2021. It is now read-only.

Sanitize checkargs #542

Closed
vkuznet opened this issue Oct 7, 2010 · 1 comment
Closed

Sanitize checkargs #542

vkuznet opened this issue Oct 7, 2010 · 1 comment
Assignees

Comments

@vkuznet
Copy link
Contributor

vkuznet commented Oct 7, 2010

Thank you for adding checkargs to verify parameters. It has a few flaws I'd like to see fixed:
You don't use what you verify. Some arguments are casted to strings (str(x)) before checking. You should instead verify what you will use.
You should type check all arguments for reasons above. A keyword argument can be None (not given), a string (given once), or a list (if given several times).
Contents of many, but not all arguments are checked. I didn't see any additional checking added for remaining arguments elsewhere so it looks like several vulnerabilities remain. You should always sanitise all arguments. Even if the argument is free form input, you can often make sure it only consists of certain legitimate characters (e.g. letters only).
Failure to verify arguments should raise an exception.
Failure to check an argument should not return the argument value back to caller. This is unsafe; you don't know what the value contains, and you just determined it's not valid. Returning the value to caller can be used to create XSS and other attacks. My general preference is to never return anything to the caller - you simply return suitable HTTP status code.
It's not sanitising the HTTP method; note that 'method' keyword argument is not the same as the request method!

@vkuznet
Copy link
Contributor Author

vkuznet commented Oct 13, 2010

valya: (In 41f5232) DAS web enhancements: fixes #525, #542, #455.

Signed-off-by: Valentin Kuznetsov vkuznet@gmail.com

@ghost ghost assigned vkuznet Jul 24, 2012
This was referenced Jul 24, 2012
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant