Closed alexander-haller closed 1 month ago
alexander-haller
commented
1 month ago but DoS attack is possible
only in the context of the user running elapi on his system - correct? (if so we should still change it but it does not seems a practical problem to me)
alexander-haller
commented
1 month ago In GitLab by @mhxion on Jun 24, 2024, 13:09
only in the context of the user running elapi on his system - correct?
Yup. I know, this is not a security issue in the sense that the attacker already needs to have access to the server.
I probably should've used a different term for security tag here. By a security issue, I meant "security of elAPI" itself. elAPI has a lot of entry points for receiving data from the outside (like elapi.yml, --OC, --data), and in all cases elAPI should only parse the given text without ever executing anything. Previously, Python documentation seems to have mistakenly claimed that using ast.literal_eval was safe which was misleading, as we learned just now it was not "completely safe".
Edit: I just did a test now with elapi post users -d "'*' * 1000000" --get-loc. It just throws an error which is not necessarily something that's really bad. I shouldn't have used ast.literal_eval for our purposes anyway.
alexander-haller
commented
1 month ago In GitLab by @mhxion on Jun 24, 2024, 13:19
Not a security issue really, if it just throws an error. Phew!
$ elapi post users -d "'*' * 1000000" --get-loc
╭─────────────────────────────── Traceback (most recent call last) ────────────────────────────────╮
│ /Users/culdesac/.local/pipx/venvs/venvs/elapi/lib/python3.12/site-packages/elapi/cli/elapi.py:37 │
│ 5 in post │
│ │
│ 372 │ │ raise typer.Exit(1) │
│ 373 │ # if json_: │
│ 374 │ try: │
│ ❱ 375 │ │ data: dict = ast.literal_eval(json_) │
│ 376 │ except SyntaxError: │
│ 377 │ │ logger.critical( │
│ 378 │ │ │ f"Error: Given value with --data has caused a syntax error. --data only supp │
│ │
│ /opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12 │
│ /ast.py:112 in literal_eval │
│ │
│ 109 │ │ │ │ else: │
│ 110 │ │ │ │ │ return left - right │
│ 111 │ │ return _convert_signed_num(node) │
│ ❱ 112 │ return _convert(node_or_string) │
│ 113 │
│ 114 │
│ 115 def dump(node, annotate_fields=True, include_attributes=False, *, indent=None): │
│ │
│ /opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12 │
│ /ast.py:111 in _convert │
│ │
│ 108 │ │ │ │ │ return left + right │
│ 109 │ │ │ │ else: │
│ 110 │ │ │ │ │ return left - right │
│ ❱ 111 │ │ return _convert_signed_num(node) │
│ 112 │ return _convert(node_or_string) │
│ 113 │
│ 114 │
│ │
│ /opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12 │
│ /ast.py:85 in _convert_signed_num │
│ │
│ 82 │ │ │ │ return + operand │
│ 83 │ │ │ else: │
│ 84 │ │ │ │ return - operand │
│ ❱ 85 │ │ return _convert_num(node) │
│ 86 │ def _convert(node): │
│ 87 │ │ if isinstance(node, Constant): │
│ 88 │ │ │ return node.value │
│ │
│ /opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12 │
│ /ast.py:76 in _convert_num │
│ │
│ 73 │ │ raise ValueError(msg + f': {node!r}') │
│ 74 │ def _convert_num(node): │
│ 75 │ │ if not isinstance(node, Constant) or type(node.value) not in (int, float, comple │
│ ❱ 76 │ │ │ _raise_malformed_node(node) │
│ 77 │ │ return node.value │
│ 78 │ def _convert_signed_num(node): │
│ 79 │ │ if isinstance(node, UnaryOp) and isinstance(node.op, (UAdd, USub)): │
│ │
│ /opt/homebrew/Cellar/python@3.12/3.12.4/Frameworks/Python.framework/Versions/3.12/lib/python3.12 │
│ /ast.py:73 in _raise_malformed_node │
│ │
│ 70 │ │ msg = "malformed node or string" │
│ 71 │ │ if lno := getattr(node, 'lineno', None): │
│ 72 │ │ │ msg += f' on line {lno}' │
│ ❱ 73 │ │ raise ValueError(msg + f': {node!r}') │
│ 74 │ def _convert_num(node): │
│ 75 │ │ if not isinstance(node, Constant) or type(node.value) not in (int, float, comple │
│ 76 │ │ │ _raise_malformed_node(node) │
╰──────────────────────────────────────────────────────────────────────────────────────────────────╯
ValueError: malformed node or string on line 1: <ast.BinOp object at 0x105152290>
In GitLab by @mhxion on Jun 24, 2024, 24:22
Python documentation for
ast.literal_evalhas formerly declared the AST parser method safe, but now:We should no longer use
ast.literal_evalthat we used for--query,--data,--headers,--file. Arbitrary code execution remain impossible, but DoS attack is possible. See here.Note: I forgot to mention this bug in the meeting.