diff --git a/CHANGELOG.md b/CHANGELOG.md index 7591df1008..dca19077b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,10 @@ This project adheres to [Semantic Versioning](https://semver.org/). ## UNRELEASED ### Changed +- [#1531](https://github.com/plotly/dash/pull/1531) Update the format of the docstrings to make them easier to read in the reference pages of Dash Docs and in the console. This also addresses [#1205](https://github.com/plotly/dash/issues/1205) -- [#1531](https://github.com/plotly/dash/pull/1531). Updates the format of the docstrings to make them easier to read in - the reference pages of Dash Docs and in the console. This also addresses [#1205](https://github.com/plotly/dash/issues/1205) - +### Fixed +- [#1546](https://github.com/plotly/dash/pull/1546) Validate callback request `outputs` vs `output` to avoid a perceived security issue. ## [1.19.0] - 2021-01-19 diff --git a/dash/_validate.py b/dash/_validate.py index d06856bc8c..71fef426c4 100644 --- a/dash/_validate.py +++ b/dash/_validate.py @@ -111,6 +111,26 @@ def validate_id_string(arg): ) +def validate_output_spec(output, output_spec, Output): + """ + This validation is for security and internal debugging, not for users, + so the messages are not intended to be clear. + `output` comes from the callback definition, `output_spec` from the request. + """ + if not isinstance(output, (list, tuple)): + output, output_spec = [output], [output_spec] + elif len(output) != len(output_spec): + raise exceptions.CallbackException("Wrong length output_spec") + + for outi, speci in zip(output, output_spec): + speci_list = speci if isinstance(speci, (list, tuple)) else [speci] + for specij in speci_list: + if not Output(specij["id"], specij["property"]) == outi: + raise exceptions.CallbackException( + "Output does not match callback definition" + ) + + def validate_multi_return(outputs_list, output_value, callback_id): if not isinstance(output_value, (list, tuple)): raise exceptions.InvalidCallbackReturnValue( diff --git a/dash/dash.py b/dash/dash.py index 34f6652bb7..16417345d5 100644 --- a/dash/dash.py +++ b/dash/dash.py @@ -27,7 +27,7 @@ from .fingerprint import build_fingerprint, check_fingerprint from .resources import Scripts, Css -from .dependencies import handle_callback_args +from .dependencies import handle_callback_args, Output from .development.base_component import ComponentRegistry from .exceptions import PreventUpdate, InvalidResourceError, ProxyError from .version import __version__ @@ -1004,6 +1004,7 @@ def wrap_func(func): @wraps(func) def add_context(*args, **kwargs): output_spec = kwargs.pop("outputs_list") + _validate.validate_output_spec(output, output_spec, Output) # don't touch the comment on the next line - used by debugger output_value = func(*args, **kwargs) # %% callback invoked %% diff --git a/tests/integration/callbacks/test_malformed_request.py b/tests/integration/callbacks/test_malformed_request.py new file mode 100644 index 0000000000..c815fc1b9d --- /dev/null +++ b/tests/integration/callbacks/test_malformed_request.py @@ -0,0 +1,59 @@ +import requests + + +import dash_core_components as dcc +import dash_html_components as html +import dash +from dash.dependencies import Input, Output + + +def test_cbmf001_bad_output_outputs(dash_thread_server): + app = dash.Dash(__name__) + app.layout = html.Div( + [ + dcc.Input(id="i", value="initial value"), + html.Div(html.Div([1.5, None, "string", html.Div(id="o1")])), + ] + ) + + @app.callback(Output("o1", "children"), [Input("i", "value")]) + def update_output(value): + return value + + dash_thread_server(app) + + # first a good request + response = requests.post( + dash_thread_server.url + "/_dash-update-component", + json=dict( + output="o1.children", + outputs={"id": "o1", "property": "children"}, + inputs=[{"id": "i", "property": "value", "value": 9}], + changedPropIds=["i.value"], + ), + ) + assert response.status_code == 200 + assert '"o1": {"children": 9}' in response.text + + # now some bad ones + outspecs = [ + {"output": "o1.nope", "outputs": {"id": "o1", "property": "nope"}}, + {"output": "o1.children", "outputs": {"id": "o1", "property": "nope"}}, + {"output": "o1.nope", "outputs": {"id": "o1", "property": "children"}}, + {"output": "o1.children", "outputs": {"id": "nope", "property": "children"}}, + {"output": "nope.children", "outputs": {"id": "nope", "property": "children"}}, + ] + for outspeci in outspecs: + response = requests.post( + dash_thread_server.url + "/_dash-update-component", + json=dict( + inputs=[{"id": "i", "property": "value", "value": 9}], + changedPropIds=["i.value"], + **outspeci + ), + ) + assert response.status_code == 500 + assert "o1" not in response.text + assert "children" not in response.text + assert "nope" not in response.text + assert "500 Internal Server Error" in response.text diff --git a/tests/integration/renderer/test_iframe.py b/tests/integration/renderer/test_iframe.py index 189209f4a1..22ed40bb0a 100644 --- a/tests/integration/renderer/test_iframe.py +++ b/tests/integration/renderer/test_iframe.py @@ -1,5 +1,3 @@ -from multiprocessing import Value - import dash from dash.dependencies import Input, Output from dash.exceptions import PreventUpdate @@ -9,7 +7,6 @@ def test_rdif001_sandbox_allow_scripts(dash_duo): app = dash.Dash(__name__) - call_count = Value("i") N_OUTPUTS = 50 @@ -26,7 +23,6 @@ def update_output(n_clicks): if n_clicks is None: raise PreventUpdate - call_count.value += 1 return ["{}={}".format(i, i + n_clicks) for i in range(N_OUTPUTS)] @app.server.after_request @@ -39,6 +35,11 @@ def apply_cors(response): dash_duo.start_server(app) + dash_duo.find_element("#btn").click() + dash_duo.wait_for_element("#output-0").text == "0=1" + + assert dash_duo.get_logs() == [] + iframe = """ @@ -53,8 +54,9 @@ def apply_cors(response): dash_duo.driver.switch_to.frame(0) - dash_duo.wait_for_element("#output-0") - dash_duo.wait_for_element_by_id("btn").click() - dash_duo.wait_for_element("#output-0").text == "0=1" + assert dash_duo.get_logs() == [] + + dash_duo.find_element("#btn").click() + dash_duo.wait_for_text_to_equal("#output-0", "0=1") assert dash_duo.get_logs() == []