zopefoundation / Zope

Zope is an open-source web application server.
https://zope.readthedocs.io
Other
352 stars 99 forks source link

TypeError: HTTPRequest fails when processing input text records that need tainting and their position is after file type records #1095

Closed xispa closed 1 year ago

xispa commented 1 year ago

BUG/PROBLEM REPORT / FEATURE REQUEST

A TypeError: can't pickle cStringIO.StringO objects is raised when ZPublisher processes a request input record with a value that contains a character that needs to be tainted (e.g. "<") and when a record from "file" type has been processed previously.

When a value from a record should be tainted, ZPublisher does it, but it also does a deepcopyof the records processed earlier here: https://github.com/zopefoundation/Zope/blob/53d8f7e0d2e30a66984b1380dbe52c1fff764139/src/ZPublisher/HTTPRequest.py#L725-L726 . Thus, if one (or more) of these records processed earlier is from file type, deepcopy cannot do the job and the following traceback arises:

Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 371, in publish_module
  Module ZPublisher.WSGIPublisher, line 232, in publish
  Module ZPublisher.HTTPRequest, line 737, in processInputs
  Module copy, line 163, in deepcopy
  Module copy, line 230, in _deepcopy_list
  Module copy, line 190, in deepcopy
  Module copy, line 334, in _reconstruct
  Module copy, line 163, in deepcopy
  Module copy, line 257, in _deepcopy_dict
  Module copy, line 190, in deepcopy
  Module copy, line 334, in _reconstruct
  Module copy, line 163, in deepcopy
  Module copy, line 257, in _deepcopy_dict
  Module copy, line 182, in deepcopy
TypeError: can't pickle cStringIO.StringO objects

As a result, the base error screen from plone "We’re sorry, but there seems to be an error…" is rendered, but without any error or traceback information.

What I did:

Tried to submit a form made of the following:

<form name="set-analysis-conditions-form"
                class="form"
                method="POST"
                enctype="multipart/form-data"
                tal:attributes="action string:${here/absolute_url}/set_analysis_conditions">

            <table class="table table-sm small">
              <colgroup>
                <col style="width:20%"/>
              </colgroup>
              <tal:condition repeat="condition python:view.get_conditions()">
                <tr tal:define="required python:condition.get('required') == 'on';
                                is_checkbox python:condition.get('type') == 'checkbox';
                                required python:required and not is_checkbox;">
                  <td>
                    <label class="formQuestion">
                      <span tal:content="structure condition/title"/>
                      <span class="fieldRequired"
                            tal:condition="required"
                            i18n:attributes="title title_required;"
                            title="Required">&nbsp;</span>
                      <span class="formHelp discreet help-block-small"
                            tal:content="structure condition/description|nothing"/>
                    </label>
                  </td>
                  <td tal:define="options condition/options|nothing">

                    <!-- Hidden fields to keep original conditions info -->
                    <input type="hidden" name="conditions.title:records"
                           tal:attributes="value condition/title"/>
                    <input type="hidden" name="conditions.description:records"
                           tal:attributes="value condition/description"/>
                    <input type="hidden" name="conditions.choices:records"
                           tal:attributes="value condition/choices"/>
                    <input type="hidden" name="conditions.uid:records"
                           tal:attributes="value condition/uid"/>
                    <input type="hidden" name="conditions.type:records"
                           tal:attributes="value condition/type"/>
                    <input type="hidden" name="conditions.required:records"
                           tal:attributes="value condition/required"/>
                    <input type="hidden" name="conditions.attachment:records"
                           tal:attributes="value condition/attachment/uid|nothing"/>

                    <select tal:condition="options"
                            tal:define="value condition/value|nothing"
                            name="conditions.value:records">
                      <option tal:condition="not:required"/>
                      <tal:option repeat="option options">
                        <option tal:content="option"
                                tal:condition="python:option == value"
                                selected="selected"/>
                        <option tal:content="option"
                                tal:condition="python:option != value" />
                      </tal:option>
                    </select>

                    <tal:non_select condition="not:options">
                      <input tal:attributes="type condition/type;
                                             value condition/value"
                             tal:condition="required"
                             required="required"
                             name="conditions.value:records"/>

                      <input tal:attributes="type condition/type;
                                             value condition/value"
                             tal:condition="not:required"
                             name="conditions.value:records"/>
                    </tal:non_select>

                    <div tal:define="attachment condition/attachment|nothing"
                         tal:condition="attachment" class="mt-2">

                      <a tal:content="attachment/filename"
                         tal:attributes="href attachment/download_url"/>
                    </div>

                  </td>
                </tr>
              </tal:condition>
            </table>

            <div class="form-group mt-2">
              <input class="btn btn-sm btn-primary"
                     type="submit"
                     name="set_conditions"
                     i18n:attributes="value"
                     value="Set Conditions" />
            </div>

            <!-- hidden fields -->
            <input type="hidden" name="submitted" value="1" />
            <input type="hidden" name="uid"
                   tal:attributes="value python:view.get_uid_from_request()" />
            <input tal:replace="structure context/@@authenticator/authenticator"/>

          </form>

What I expect to happen:

No traceback, records are processed correctly

What actually happened:

Traceback (innermost last):
  Module ZPublisher.WSGIPublisher, line 162, in transaction_pubevents
  Module ZPublisher.WSGIPublisher, line 371, in publish_module
  Module ZPublisher.WSGIPublisher, line 232, in publish
  Module ZPublisher.HTTPRequest, line 737, in processInputs
  Module copy, line 163, in deepcopy
  Module copy, line 230, in _deepcopy_list
  Module copy, line 190, in deepcopy
  Module copy, line 334, in _reconstruct
  Module copy, line 163, in deepcopy
  Module copy, line 257, in _deepcopy_dict
  Module copy, line 190, in deepcopy
  Module copy, line 334, in _reconstruct
  Module copy, line 163, in deepcopy
  Module copy, line 257, in _deepcopy_dict
  Module copy, line 182, in deepcopy
TypeError: can't pickle cStringIO.StringO objects

PDB session

TypeError: can't pickle cStringIO.StringO objects
(Pdb++) where
[...]
-> self.execute()
[6]   /home/senaite/dev/buildout-cache/eggs/waitress-1.4.4-py2.7.egg/waitress/task.py(441)execute()
-> app_iter = self.channel.server.application(environ, start_response)
[7]   /home/senaite/dev/buildout-cache/eggs/Paste-3.5.0-py2.7.egg/paste/translogger.py(69)__call__()
-> return self.application(environ, replacement_start_response)
[8]   /home/senaite/dev/buildout-cache/eggs/Zope-4.8.2-py2.7.egg/ZPublisher/httpexceptions.py(30)__call__()
-> return self.application(environ, start_response)
[9]   /home/senaite/dev/buildout-cache/eggs/Zope-4.8.2-py2.7.egg/ZPublisher/WSGIPublisher.py(371)publish_module()
-> response = _publish(request, new_mod_info)
[10]   /home/senaite/dev/buildout-cache/eggs/Zope-4.8.2-py2.7.egg/ZPublisher/WSGIPublisher.py(232)publish()
-> request.processInputs()
[11] > /home/senaite/dev/buildout-cache/eggs/Zope-4.8.2-py2.7.egg/ZPublisher/HTTPRequest.py(738)processInputs()
-> reclist)
[12]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(163)deepcopy()
-> y = copier(x, memo)
[13]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(230)_deepcopy_list()
-> y.append(deepcopy(a, memo))
[14]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(190)deepcopy()
-> y = _reconstruct(x, rv, 1, memo)
[15]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(334)_reconstruct()
-> state = deepcopy(state, memo)
[16]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(163)deepcopy()
-> y = copier(x, memo)
[17]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(257)_deepcopy_dict()
-> y[deepcopy(key, memo)] = deepcopy(value, memo)
[18]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(190)deepcopy()
-> y = _reconstruct(x, rv, 1, memo)
[19]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(334)_reconstruct()
-> state = deepcopy(state, memo)
[20]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(163)deepcopy()
-> y = copier(x, memo)
[21]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(257)_deepcopy_dict()
-> y[deepcopy(key, memo)] = deepcopy(value, memo)
[22]   /home/senaite/dev/Python-2.7/lib/python2.7/copy.py(182)deepcopy()
-> rv = reductor(2)
(Pdb++) key
'conditions'
(Pdb++) item
'<'
(Pdb++) reclist[-1]
{'attachment': '', 'choices': '', 'description': '', 'required': '', 'title': 'Comentarios TGA', 'type': 'text', 'uid': '3939c785fcce47c9a3425d7e1d28aca2'}
(Pdb++) reclist[-2]
{'attachment': '', 'choices': '', 'description': 'Si el an\xc3\xa1lisis no se ajusta al esquema anterior, adjuntar fichero con las condiciones de ensayo', 'required': '', 'title': 'Fichero adjunto', 'type': 'file', 'uid': '3939c785fcce47c9a3425d7e1d28aca2', 'value': <ZPublisher.HTTPRequest.FileUpload object at 0x7fd973388b90>}

What version of Python and Zope/Addons I am using:

d-maurer commented 1 year ago

Jordi Puiggené wrote at 2023-1-18 02:39 -0800:

A TypeError: can't pickle cStringIO.StringO objects is raised when ZPublisher processes a request input record with a value that contains a character that needs to be tainted (e.g. "<") and when a record from "file" type has been processed previously.

Tainting is an old security feature for DTML: when form data or cookies are accessed implicitly via the request object, they are (HTML) quoted on DTML rendering if they contain unsafe characters.

The tainting logic is incredibly complex and your bug report reveals one of its holes. I have tried to streamline it in "https://github.com/zopefoundation/Zope/pull/648", but this is not yet merged (and might have the same problem you have observed).

Page templates no longer rely on tainting logic; they quote by default and unquoted rendering must be called for explicitly (and this hopefully is not done for things involving user input).

If you do not process untrusted user input with DTML objects, you could disable the tainting logic giving the envvar "ZOPE_DTML_REQUEST_AUTOQUOTE" e.g. the value "0". This should avoid the bug you have reported.

Note that Zope's management interface (--> "ZMI") partially still uses DTML. Thus, if you disable tainting, you likely must restrict the use of large parts of the ZMI to trusted users. A good starting point would be to restrict the permission View management screens (or similarly spelled) to Manager.

d-maurer commented 1 year ago

@xispa You could try the version from #1097.

xispa commented 1 year ago

Sure, let me try

xispa commented 1 year ago

Excellent! :confetti_ball: it works properly now. Thank you very much @d-maurer for your fast response, we really appreciate it!