search

scipy / SciPyCentral

SciPy Central
http://scipy-central.org
BSD 3-Clause "New" or "Revised" License
16 stars 12 forks source link

Do not include or use 'pk' field in forms #117

Closed pv closed 11 years ago

pv commented 12 years ago

Including primary keys in forms offers possible attack vectors (one of which AFAIK would be possible currently). In the case here, they are not necessary because the component is selected via the URLs (and that is a better place for authorization, and it is presently controlled on that level). This should address the issue in #101

The second fix asks for authorization when validating submissions.

pv commented 11 years ago

@ksurya: if you have time, can you check this one out (i.e., if it doesn't outrageously break anything, it should be OK)? Would be nice to have this merged...

ksurya commented 11 years ago

Okay, I will try to look into it at the weekend and comment. Have to look around what you have changed and how the security hole is created if pk is used..

ksurya commented 11 years ago

If I am not wrong, you basically removed primary key of submission in the form and replaced it with submission object for creating revisions? I just went through the code! let you know how its actually working.

Thanks

pv commented 11 years ago

Rebased.

@ksurya: yes, it's safer to pass the submission object around (Django has already checked that the user has the authorization to edit it), than to pull out pk from user-submitted data (which can be whatever).

pv commented 11 years ago

Ok, I just merged it as it had been sitting around a year already. If this breaks something, please just shout and I'll try to fix it :)

ksurya commented 11 years ago

I am sorry, I have not checked it yet.. I am just running behind the GSoC deadlines :) Will definitely check this out and let you know.. as far as I see there is no problem with it.

But I have one question - getting pk value from Forms or as a parameter from Url is not advised? Much of django uses that way! I would be nice if you can direct to some paper or site where its described (I have to confess that I don't know about it)

pv commented 11 years ago

Getting pk from url/forms is OK, but only if you check that the user has permissions to edit the object in question.

That the user matches the pk from the URL is checked in edit_submissions, but there was no check on the pk extracted from the form.