From b866bd2a565d5cc87ad0f07b21448aeb78119ebf Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Fri, 1 Sep 2023 18:22:41 +0200 Subject: [PATCH] api: encode x-error-msg as JSON - so it does not contain or When the server raises an exception in a HTTP request handler context, more often than not, the exception is sent back to the client in the body. Additionally, the message of the exception (if any), is also copied as is in a x-error-msg HTTP header. That said, HTTP headers must obey strict rules. The "\r\n" sequence indicate the end of the current HTTP header. When using aiohttp, the library rejects any header that has a "\r" or "\n" in its value: ValueError: Newline or carriage return character detected in HTTP status message or header. This is a potential security issue. As an example, any curtin.util.ProcessExecutionError exception will contain "\n" characters when converted into a string. We now encode the error message as JSON before copying it in the HTTP header. Signed-off-by: Olivier Gayot --- subiquity/common/api/server.py | 6 +++++- subiquity/common/api/tests/test_server.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/subiquity/common/api/server.py b/subiquity/common/api/server.py index 2420f906..3e70ccf2 100644 --- a/subiquity/common/api/server.py +++ b/subiquity/common/api/server.py @@ -14,6 +14,7 @@ # along with this program. If not, see . import inspect +import json import logging import os import traceback @@ -173,7 +174,10 @@ def _make_handler( headers={ "x-status": "error", "x-error-type": type(exc).__name__, - "x-error-msg": str(exc), + # aiohttp will reject a header if its value contains a + # "\r" or "\n" character. By using compact JSON, we + # ensure those characters are escaped. + "x-error-msg": json.dumps(str(exc), indent=None), }, ) resp["exception"] = exc diff --git a/subiquity/common/api/tests/test_server.py b/subiquity/common/api/tests/test_server.py index 460541f6..0dcc74b5 100644 --- a/subiquity/common/api/tests/test_server.py +++ b/subiquity/common/api/tests/test_server.py @@ -122,7 +122,7 @@ class TestBind(unittest.IsolatedAsyncioTestCase): self.assertEqual(resp.headers["x-status"], "error") self.assertEqual(resp.headers["x-error-type"], "TypeError") self.assertEqual( - resp.headers["x-error-msg"], 'missing required argument "arg"' + resp.headers["x-error-msg"], '"missing required argument \\"arg\\""' ) async def test_error(self):