search

FNNDSC / ChRIS_ultron_backEnd

Backend for ChRIS
https://fnndsc.github.io/ChRIS_ultron_backEnd
MIT License
31 stars 100 forks source link

Assuming type of unsanitized user input #382

Open jennydaman opened 2 years ago

jennydaman commented 2 years ago

https://github.com/FNNDSC/ChRIS_ultron_backEnd/blob/3ee492dfa699b6394b19a6458fd511bda2824e09/chris_backend/plugininstances/views.py#L38

Here, self.request.data is unsanitized user input and we are assuming it to be of type dict. Poorly formed user input can cause a 500 Internal Server error, with exceptions such as:

AttributeError: 'str' object has no attribute 'pop'

How To Reproduce

curl -i -u chris:chris1234 localhost:8000/api/v1/plugins/3/instances/ --data 0

Where the value for --data is arbitrary, it can even be a number, empty string, null, ... so long as it does not conform to the expected schema.

Expected Result

400 bad request

Actual Result

500 Internal Server Error, some type-related error internally

fortune-max commented 2 years ago

Review pull request here.

jennydaman commented 2 years ago

As mentioned in https://github.com/FNNDSC/ChRIS_ultron_backEnd/pull/388/files#r863843479 there is also unrelated trouble with variable mutability. Very good catch @fortune-max!

This problem shows up in multiple places so we should rethink the codebase a bit and figure out a better solution.

$ rg 'self.request.data.pop'
chris_backend/feeds/views.py
360:        username = self.request.data.pop('owner')

chris_backend/pacsfiles/views.py
45:        self.request.data.pop('fname', None)

chris_backend/servicefiles/views.py
40:        self.request.data.pop('fname', None)

chris_backend/plugininstances/views.py
38:        self.request.data.pop('status', None)
cornelia247 commented 2 years ago

hey @jennydaman I have gone through all the comments concerning this issue and I am interested in fixing up the remaining codebase, please assign to me

cornelia247 commented 2 years ago

Hello if assigned please comment or/and merge https://github.com/FNNDSC/ChRIS_ultron_backEnd/pull/442

majorchork commented 1 year ago

hello @jennydaman I'm a potential Outreachy intern for the 2023 summer cohort, I'd like to be assigned this issue, please.

majorchork commented 1 year ago

Hello @jennydaman just checking in in case you missed my earlier message