Skip to content

Commit

Permalink
Fix: plug security issue partition system files via include (#3908)
Browse files Browse the repository at this point in the history
#### Summary

A recent security review showed that it was possible to partition
arbitrary local files in cases where the filetype supports an "include"
functionality that brings in the content of files external to the
partitioned file. This affects `rst` and `org` files.

#### Fix

This PR fixes the above issue by passing the parameter `sandbox=True` in
all cases where `pypandoc.convert_file` is called.

Note I also added the parameter to a call to this method in the ODT
code. I haven't investigated whether there was a security issue with ODT
files, but it seems better to use pandoc in sandbox mode given the
security issues we know about.

#### Testing

To verify that the tests that are added with this PR find the relevant
issue:
- Remove the `sandbox=True` text from
`unstructured/file_utils/file_conversion.py` line 17.
- Run the tests
`test_unstructured.partition.test_rst.test_rst_wont_include_external_files`
and
`test_unstructured.partition.test_org.test_org_wont_include_external_files`.
Both should fail due to the partitioning containing the word "wombat",
which only appears in a file external to the partitioned file.
- Add the parameter back in, and the tests pass.
  • Loading branch information
qued authored Feb 6, 2025
1 parent 5852260 commit b10379c
Show file tree
Hide file tree
Showing 10 changed files with 114 additions and 8 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
## 0.16.20

### Enhancements

### Features

### Fixes
- **Fix a security issue where rst and org files could read files in the local filesystem**. Certain filetypes could 'include' or 'import' local files into their content, allowing partitioning of arbitrary files from the local filesystem. Partitioning of these files is now sandboxed.

## 0.16.19

### Enhancements
Expand Down
29 changes: 29 additions & 0 deletions example-docs/README-w-include.org
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#+INCLUDE: "file_we_dont_want_imported"

* Example Docs

The sample docs directory contains the following files:

- ~example-10k.html~ - A 10-K SEC filing in HTML format
- ~layout-parser-paper.pdf~ - A PDF copy of the layout parser paper
- ~factbook.xml~ / ~factbook.xsl~ - Example XML/XLS files that you
can use to test stylesheets

These documents can be used to test out the parsers in the library. In
addition, here are instructions for pulling in some sample docs that are
too big to store in the repo.

** XBRL 10-K

You can get an example 10-K in inline XBRL format using the following
~curl~. Note, you need to have the user agent set in the header or the
SEC site will reject your request.

#+BEGIN_SRC bash

curl -O \
-A '${organization} ${email}'
https://www.sec.gov/Archives/edgar/data/311094/000117184321001344/0001171843-21-001344.txt
#+END_SRC

You can parse this document using the HTML parser.
30 changes: 30 additions & 0 deletions example-docs/README-w-include.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
.. include:: file_we_dont_want_imported

Example Docs
------------

The sample docs directory contains the following files:

- ``example-10k.html`` - A 10-K SEC filing in HTML format
- ``layout-parser-paper.pdf`` - A PDF copy of the layout parser paper
- ``factbook.xml``/``factbook.xsl`` - Example XML/XLS files that you
can use to test stylesheets

These documents can be used to test out the parsers in the library. In
addition, here are instructions for pulling in some sample docs that are
too big to store in the repo.

XBRL 10-K
^^^^^^^^^

You can get an example 10-K in inline XBRL format using the following
``curl``. Note, you need to have the user agent set in the header or the
SEC site will reject your request.

.. code:: bash
curl -O \
-A '${organization} ${email}'
https://www.sec.gov/Archives/edgar/data/311094/000117184321001344/0001171843-21-001344.txt
You can parse this document using the HTML parser.
1 change: 1 addition & 0 deletions example-docs/file_we_dont_want_imported
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
wombat
20 changes: 19 additions & 1 deletion test_unstructured/partition/test_org.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
from __future__ import annotations

from pathlib import Path

from pytest_mock import MockFixture

from test_unstructured.unit_utils import assert_round_trips_through_JSON, example_doc_path
from test_unstructured.unit_utils import (
assert_round_trips_through_JSON,
example_doc_path,
find_text_in_elements,
)
from unstructured.chunking.title import chunk_by_title
from unstructured.documents.elements import Title
from unstructured.partition.org import partition_org
Expand Down Expand Up @@ -138,3 +144,15 @@ def test_partition_org_respects_detect_language_per_element():
)
langs = [element.metadata.languages for element in elements]
assert langs == [["eng"], ["spa", "eng"], ["eng"], ["eng"], ["spa"]]


def test_org_wont_include_external_files():
# Make sure our import file is in place (otherwise the import fails silently and test passes)
assert Path(example_doc_path("file_we_dont_want_imported")).exists()
elements = partition_org(example_doc_path("README-w-include.org"))
# The partition should contain some elements
assert elements
# We find something we expect to find from file we partitioned directly
assert find_text_in_elements("instructions", elements)
# But we don't find something from the file included within the file we partitioned directly
assert not find_text_in_elements("wombat", elements)
20 changes: 19 additions & 1 deletion test_unstructured/partition/test_rst.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
from __future__ import annotations

from pathlib import Path

from pytest_mock import MockFixture

from test_unstructured.unit_utils import assert_round_trips_through_JSON, example_doc_path
from test_unstructured.unit_utils import (
assert_round_trips_through_JSON,
example_doc_path,
find_text_in_elements,
)
from unstructured.chunking.title import chunk_by_title
from unstructured.documents.elements import Title
from unstructured.partition.rst import partition_rst
Expand Down Expand Up @@ -120,3 +126,15 @@ def test_partition_rst_respects_detect_language_per_element():
)
langs = [element.metadata.languages for element in elements]
assert langs == [["eng"], ["spa", "eng"], ["eng"], ["eng"], ["spa"]]


def test_rst_wont_include_external_files():
# Make sure our import file is in place (otherwise the import fails silently and test passes)
assert Path(example_doc_path("file_we_dont_want_imported")).exists()
elements = partition_rst(example_doc_path("README-w-include.rst"))
# The partition should contain some elements
assert elements
# We find something we expect to find from file we partitioned directly
assert find_text_in_elements("instructions", elements)
# But we don't find something from the file included within the file we partitioned directly
assert not find_text_in_elements("wombat", elements)
4 changes: 4 additions & 0 deletions test_unstructured/unit_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,7 @@ def var_mock(request: FixtureRequest, q_var_name: str, **kwargs: Any) -> Mock:
_patch = patch(q_var_name, **kwargs)
request.addfinalizer(_patch.stop)
return _patch.start()


def find_text_in_elements(text: str, elements: List[Element]):
return any(el.text.find(text) != -1 for el in elements)
2 changes: 1 addition & 1 deletion unstructured/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.16.19" # pragma: no cover
__version__ = "0.16.20" # pragma: no cover
2 changes: 1 addition & 1 deletion unstructured/file_utils/file_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def convert_file_to_text(filename: str, source_format: str, target_format: str)
import pypandoc

try:
text = pypandoc.convert_file(filename, target_format, format=source_format)
text = pypandoc.convert_file(filename, target_format, format=source_format, sandbox=True)
except FileNotFoundError as err:
msg = (
f"Error converting the file to text. Ensure you have the pandoc package installed on"
Expand Down
5 changes: 1 addition & 4 deletions unstructured/partition/odt.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,7 @@ def _convert_odt_to_docx(
import pypandoc

pypandoc.convert_file(
source_file_path,
"docx",
format="odt",
outputfile=target_docx_path,
source_file_path, "docx", format="odt", outputfile=target_docx_path, sandbox=True
)

return target_docx_path

0 comments on commit b10379c

Please sign in to comment.