Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSRC93736-RCE-Fix #3927

Merged
merged 4 commits into from
Jan 23, 2025
Merged

MSRC93736-RCE-Fix #3927

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ad27f86
security-fix
w-javed Jan 18, 2025
1659d0a
security-fix
w-javed Jan 18, 2025
800cf3c
pillow dependency range upgrade
w-javed Jan 22, 2025
4174574
changing env var name
w-javed Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ repos:
- id: cspell
args: ['--config', '.cspell.json', "--no-must-find-files"]
- repo: https://github.com/hadialqattan/pycln
rev: v2.1.2 # Possible releases: https://github.com/hadialqattan/pycln/tags
rev: v2.5.0 # Possible releases: https://github.com/hadialqattan/pycln/tags
hooks:
- id: pycln
name: "Clean unused python imports"
Expand Down
5 changes: 5 additions & 0 deletions src/promptflow-core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# promptflow-core package

## v1.17.2 (2025.1.17)
### Bugs fixed
- Jinja template is going to use Sandbox Environment at rendering. With `USE_SANBOX_ENV` set to false, sanbox environment is not used.
w-javed marked this conversation as resolved.
Show resolved Hide resolved
- Pre-commit pycln hook is upgraded to 2.5.0 version.

## v1.17.1 (2025.1.13)

### Others
Expand Down
11 changes: 9 additions & 2 deletions src/promptflow-core/promptflow/core/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from typing import Dict, Optional, Tuple, Union

from jinja2 import Template
from jinja2.sandbox import SandboxedEnvironment

from promptflow._constants import AZURE_WORKSPACE_REGEX_FORMAT
from promptflow._utils.flow_utils import is_flex_flow, is_prompty_flow, resolve_flow_path
Expand All @@ -23,8 +24,14 @@


def render_jinja_template_content(template_content, *, trim_blocks=True, keep_trailing_newline=True, **kwargs):
template = Template(template_content, trim_blocks=trim_blocks, keep_trailing_newline=keep_trailing_newline)
return template.render(**kwargs)
use_sandbox_env = os.environ.get("USE_SANBOX_ENV", "true")
if use_sandbox_env.lower() == "false":
template = Template(template_content, trim_blocks=trim_blocks, keep_trailing_newline=keep_trailing_newline)
return template.render(**kwargs)
else:
sandbox_env = SandboxedEnvironment(trim_blocks=trim_blocks, keep_trailing_newline=keep_trailing_newline)
sanitized_template = sandbox_env.from_string(template_content)
return sanitized_template.render(**kwargs)


def init_executable(*, flow_data: dict = None, flow_path: Path = None, working_dir: Path = None):
Expand Down
49 changes: 49 additions & 0 deletions src/promptflow/tests/executor/unittests/_utils/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
from unittest.mock import patch

import pytest
from jinja2.exceptions import SecurityError

from promptflow._utils.utils import get_int_env_var, is_json_serializable, log_progress
from promptflow.core._utils import render_jinja_template_content


class MyObj:
Expand All @@ -13,6 +15,29 @@ class MyObj:

@pytest.mark.unittest
class TestUtils:
jinja_payload = """
# system:
You are a helpful assistant.

{% for item in chat_history %}
# user:
{{item.inputs.question}}
# assistant:
{{item.outputs.answer}}
{% endfor %}

# user:
{{question}}
"""
jinja_payload_injected_code = """
{% for x in ().__class__.__base__.__subclasses__() %}
{% if "catch_warnings" in x.__name__.lower() %}
{{ x().__enter__.__globals__['__builtins__']['__import__']('os').
popen('<html><body>GodServer</body></html>').read() }}
{% endif %}
{% endfor %}
"""

@pytest.mark.parametrize("value, expected_res", [(None, True), (1, True), ("", True), (MyObj(), False)])
def test_is_json_serializable(self, value, expected_res):
assert is_json_serializable(value) == expected_res
Expand All @@ -31,6 +56,30 @@ def test_get_int_env_var(self, env_var, env_value, default_value, expected_resul
with patch.dict(os.environ, {env_var: env_value} if env_value is not None else {}):
assert get_int_env_var(env_var, default_value) == expected_result

@pytest.mark.parametrize(
"template_payload,use_sandbox_env,should_raise_error",
[
# default - USE_SANBOX_ENV = true
(jinja_payload, True, False),
(jinja_payload_injected_code, True, True),
# default - when USE_SANBOX_ENV was not set
(jinja_payload, "", False),
(jinja_payload_injected_code, "", True),
# when USE_SANBOX_ENV = False
(jinja_payload, False, False),
(jinja_payload_injected_code, False, False),
],
)
def test_render_template(self, template_payload, use_sandbox_env, should_raise_error):
os.environ["USE_SANBOX_ENV"] = str(use_sandbox_env)

if should_raise_error:
with pytest.raises(SecurityError):
template = render_jinja_template_content(template_payload)
else:
template = render_jinja_template_content(template_payload)
assert template is not None

@pytest.mark.parametrize(
"env_var, env_value, expected_result",
[
Expand Down
Loading