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

XSS Bypass in custom fields displayed in tables #6768

New Issue
Closed
opened 2025-12-29 19:45:11 +01:00 by adam · 8 comments
adam commented 2025-12-29 19:45:11 +01:00
Owner
Copy Link

Originally created by @0xdeadbeer on GitHub (Aug 4, 2022).

Originally assigned to: @0xdeadbeer on GitHub.

NetBox version

v3.2.7

Python version

3.10

Steps to Reproduce

  1. Create a custom text field
  2. Add it do any model (in my case this was dcim/device)
  3. Make a device record with any XSS inside the custom field
  4. Go to the devices list view /dcim/devices/

image
image
image

Expected Behavior

HTML being sanitized

Observed Behavior

The HTML is not being sanitized in the table view allowing XSS.

Originally created by @0xdeadbeer on GitHub (Aug 4, 2022). Originally assigned to: @0xdeadbeer on GitHub. ### NetBox version v3.2.7 ### Python version 3.10 ### Steps to Reproduce 1. Create a custom text field 2. Add it do any model (in my case this was dcim/device) 3. Make a device record with any XSS inside the custom field 4. Go to the devices list view `/dcim/devices/` ![image](https://user-images.githubusercontent.com/64986162/182784622-b9c5a1a5-9681-4cbb-b88c-5f64fef45622.png) ![image](https://user-images.githubusercontent.com/64986162/182784557-ae4caee5-728b-41de-a6bb-245f6197dddd.png) ![image](https://user-images.githubusercontent.com/64986162/182784752-63bd9f44-d803-4785-a04b-43bdfbcaf060.png) ### Expected Behavior HTML being sanitized ### Observed Behavior The HTML is not being sanitized in the table view allowing XSS.
adam added the type: bugstatus: accepted labels 2025-12-29 19:45:11 +01:00
adam closed this issue 2025-12-29 19:45:11 +01:00
adam commented 2025-12-29 19:45:12 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Aug 4, 2022):

I edited your report to make it more consistent with the actual issue (only the table view is affected as far as I can tell).

Issue is here:

a397ce234a/netbox/netbox/tables/columns.py (L435-L450)

As everything is marked safe, no sanitization is being done. The method should either be refactored or escape() should be used on all values.

I've marked as needs owner, so if anyone wants to own it, just give the word. Otherwise I'll get to it after my vacation.

Thanks for the report.

@kkthxbye-code commented on GitHub (Aug 4, 2022): I edited your report to make it more consistent with the actual issue (only the table view is affected as far as I can tell). Issue is here: https://github.com/netbox-community/netbox/blob/a397ce234aa0edf9d93b158d7f37fdbb548d6cf8/netbox/netbox/tables/columns.py#L435-L450 As everything is marked safe, no sanitization is being done. The method should either be refactored or `escape()` should be used on all values. I've marked as needs owner, so if anyone wants to own it, just give the word. Otherwise I'll get to it after my vacation. Thanks for the report.
adam commented 2025-12-29 19:45:12 +01:00
Author
Owner
Copy Link

@0xdeadbeer commented on GitHub (Aug 4, 2022):

I see, thanks for editing the report! Will try to be more consistent next time 😋
I'm going to give it a shot and try to solve it on my own, will give you updates if I find the vulnerability is present elsewhere or if I manage to solve the issue. Enjoy your vacation ^^

I edited your report to make it more consistent with the actual issue (only the table view is affected as far as I can tell).

Issue is here:

a397ce234a/netbox/netbox/tables/columns.py (L435-L450)

As everything is marked safe, no sanitization is being done. The method should either be refactored or escape() should be used on all values.

I've marked as needs owner, so if anyone wants to own it, just give the word. Otherwise I'll get to it after my vacation.

Thanks for the report.

@0xdeadbeer commented on GitHub (Aug 4, 2022): I see, thanks for editing the report! Will try to be more consistent next time 😋 I'm going to give it a shot and try to solve it on my own, will give you updates if I find the vulnerability is present elsewhere or if I manage to solve the issue. Enjoy your vacation ^^ > I edited your report to make it more consistent with the actual issue (only the table view is affected as far as I can tell). > > Issue is here: > > https://github.com/netbox-community/netbox/blob/a397ce234aa0edf9d93b158d7f37fdbb548d6cf8/netbox/netbox/tables/columns.py#L435-L450 > > As everything is marked safe, no sanitization is being done. The method should either be refactored or `escape()` should be used on all values. > > I've marked as needs owner, so if anyone wants to own it, just give the word. Otherwise I'll get to it after my vacation. > > Thanks for the report.
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@0xdeadbeer commented on GitHub (Aug 4, 2022):

So far fixed issue in my fork: here
I'm not sure whether I should already make a pull request or not. Either way I'll dedicate the next couple of days to double checking if the bug is present in any other locations. Peace ✌️
solved

@0xdeadbeer commented on GitHub (Aug 4, 2022): So far fixed issue in my fork: [here](https://github.com/osamu-kj/netbox/commit/f874e9932d06653e330da65d93c2fe0fd7720968) I'm not sure whether I should already make a pull request or not. Either way I'll dedicate the next couple of days to double checking if the bug is present in any other locations. Peace :v: ![solved](https://user-images.githubusercontent.com/64986162/182909439-289c922e-3030-4602-b663-beb56e062f61.png)
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Aug 4, 2022):

@osamu-kj - Thank you for contributing. There's some issue with the approach though.

f874e9932d/netbox/netbox/tables/columns.py (L2)

Should be from django.utils.html import escape. glob.escape() is for filesystem paths.

f874e9932d/netbox/netbox/tables/columns.py (L436-L441)

You can't just escape the whole lines as the non-usersupplied HTML is actually supposed to render as HTML.

It would have to be something like this instead:

if self.customfield.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value is True:
    return mark_safe('<i class="mdi mdi-check-bold text-success"></i>')
if self.customfield.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value is False:
    return mark_safe('<i class="mdi mdi-close-thick text-danger"></i>')
if self.customfield.type == CustomFieldTypeChoices.TYPE_URL:
    return mark_safe(f'<a href="{escape(value)}">{escape(value)}</a>')

The first two cases contain no user-input, so we leave those as is.

CustomFieldTypeChoices.TYPE_MULTISELECT and CustomFieldTypeChoices.TYPE_MULTIOBJECT are fine as they are. MULTISELECT is never marked safe, so that get's escaped automatically and I don't see a way to get XSS via. MULTIOBJECT. Might be wrong on the last one though.

f874e9932d/netbox/netbox/tables/columns.py (L448-L451)

Links to objects wont work if the type of the customfield is Object when doing it this way. It should be escaped inside _linkify_item instead:

@staticmethod
def _likify_item(item):
    if hasattr(item, 'get_absolute_url'):
        return f'<a href="{item.get_absolute_url()}">{item}</a>'
    return escape(item)

The default value does not need to be escaped as it is never marked safe.

With these changes all the XSS vectors "should" be gone, I haven't tested it properly though.

@kkthxbye-code commented on GitHub (Aug 4, 2022): @osamu-kj - Thank you for contributing. There's some issue with the approach though. https://github.com/osamu-kj/netbox/blob/f874e9932d06653e330da65d93c2fe0fd7720968/netbox/netbox/tables/columns.py#L2 Should be `from django.utils.html import escape`. ` glob.escape()` is for filesystem paths. https://github.com/osamu-kj/netbox/blob/f874e9932d06653e330da65d93c2fe0fd7720968/netbox/netbox/tables/columns.py#L436-L441 You can't just escape the whole lines as the non-usersupplied HTML is actually supposed to render as HTML. It would have to be something like this instead: ```python if self.customfield.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value is True: return mark_safe('<i class="mdi mdi-check-bold text-success"></i>') if self.customfield.type == CustomFieldTypeChoices.TYPE_BOOLEAN and value is False: return mark_safe('<i class="mdi mdi-close-thick text-danger"></i>') if self.customfield.type == CustomFieldTypeChoices.TYPE_URL: return mark_safe(f'<a href="{escape(value)}">{escape(value)}</a>') ``` The first two cases contain no user-input, so we leave those as is. CustomFieldTypeChoices.TYPE_MULTISELECT and CustomFieldTypeChoices.TYPE_MULTIOBJECT are fine as they are. MULTISELECT is never marked safe, so that get's escaped automatically and I don't see a way to get XSS via. MULTIOBJECT. Might be wrong on the last one though. https://github.com/osamu-kj/netbox/blob/f874e9932d06653e330da65d93c2fe0fd7720968/netbox/netbox/tables/columns.py#L448-L451 Links to objects wont work if the type of the customfield is Object when doing it this way. It should be escaped inside _linkify_item instead: ```python @staticmethod def _likify_item(item): if hasattr(item, 'get_absolute_url'): return f'<a href="{item.get_absolute_url()}">{item}</a>' return escape(item) ``` The default value does not need to be escaped as it is never marked safe. With these changes all the XSS vectors "should" be gone, I haven't tested it properly though.
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@0xdeadbeer commented on GitHub (Aug 6, 2022):

Ohh, that makes sense yeah thanks for letting me know! ^^
I've fixed it in the new commit on my fork here

Although I don't get it.. why are you escaping the item variable only on the second return? Why not escape it on both returns like so? (the get_absolute_url shouldn't be escaped though since it's gonna be a URL)

@staticmethod
def _likify_item(item):
    if hasattr(item, 'get_absolute_url'):
        return f'<a href="{item.get_absolute_url()}">{escape(item)}</a>'
    return escape(item)

Thanks for your help 😋

@0xdeadbeer commented on GitHub (Aug 6, 2022): Ohh, that makes sense yeah thanks for letting me know! ^^ I've fixed it in the new commit on my fork [here](https://github.com/osamu-kj/netbox/commit/db38ed4f19e5943bf60434f4cfc08d232dbb5e6e) Although I don't get it.. why are you escaping the `item` variable only on the second return? Why not escape it on both returns like so? (the `get_absolute_url` shouldn't be escaped though since it's gonna be a URL) ```python @staticmethod def _likify_item(item): if hasattr(item, 'get_absolute_url'): return f'<a href="{item.get_absolute_url()}">{escape(item)}</a>' return escape(item) ``` Thanks for your help :yum:
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Aug 6, 2022):

Although I don't get it.. why are you escaping the item variable only on the second return? Why not escape it on both returns like so?

I just missed it, yours should be correct. Remember to remove the print debugging in L445-447 before making a pull request.

@kkthxbye-code commented on GitHub (Aug 6, 2022): > Although I don't get it.. why are you escaping the `item` variable only on the second return? Why not escape it on both returns like so? I just missed it, yours should be correct. Remember to remove the print debugging in L445-447 before making a pull request.
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@0xdeadbeer commented on GitHub (Aug 6, 2022):

Although I don't get it.. why are you escaping the item variable only on the second return? Why not escape it on both returns like so?

I just missed it, yours should be correct. Remember to remove the print debugging in L445-447 before making a pull request.

yupp! Alright then I'm gonna make a pull request now

@0xdeadbeer commented on GitHub (Aug 6, 2022): > > Although I don't get it.. why are you escaping the `item` variable only on the second return? Why not escape it on both returns like so? > > I just missed it, yours should be correct. Remember to remove the print debugging in L445-447 before making a pull request. yupp! Alright then I'm gonna make a pull request now
adam commented 2025-12-29 19:45:13 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Aug 8, 2022):

Thanks @osamu-kj!

@jeremystretch commented on GitHub (Aug 8, 2022): Thanks @osamu-kj!
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 status: accepted 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#6768
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?