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

Invalid "address" query param causes server error #7495

New Issue
Closed
opened 2025-12-29 20:24:13 +01:00 by adam · 7 comments
adam commented 2025-12-29 20:24:13 +01:00
Owner
Copy Link

Originally created by @tyler-8 on GitHub (Jan 11, 2023).

Originally assigned to: @rmanyari on GitHub.

NetBox version

v3.2.9 (confirmed on v3.4.2 as well)

Python version

3.10

Steps to Reproduce

  1. Via API or GUI, do an HTTP GET on the IPAddress endpoint with ?address=/32 (For example: https://demo.netbox.dev/ipam/ip-addresses/?address=/32)

Expected Behavior

I would expect an empty results list, or perhaps a more gracefully handled error message like Invalid IP address.

Observed Behavior

An error is generated that references the raw SQL query that is attempted

Internal Server Error: /api/ipam/ip-addresses/

DataError at /api/ipam/ip-addresses/
invalid input syntax for type inet: "/32"
LINE 1: ..."ipam_ipaddress" WHERE "ipam_ipaddress"."address" IN ('/32')

Example from the demo site

There was a problem with your request. Please contact an administrator.

The complete exception is provided below:

<class 'django.db.utils.DataError'>

invalid input syntax for type inet: "/32"
LINE 1: ..."ipam_ipaddress" WHERE "ipam_ipaddress"."address" IN ('/32')
                                                                 ^
Python version: 3.8.10
NetBox version: 3.4.2
Originally created by @tyler-8 on GitHub (Jan 11, 2023). Originally assigned to: @rmanyari on GitHub. ### NetBox version v3.2.9 (confirmed on v3.4.2 as well) ### Python version 3.10 ### Steps to Reproduce 1. Via API or GUI, do an HTTP GET on the `IPAddress` endpoint with `?address=/32` (For example: `https://demo.netbox.dev/ipam/ip-addresses/?address=/32`) ### Expected Behavior I would expect an empty results list, or perhaps a more gracefully handled error message like `Invalid IP address`. ### Observed Behavior An error is generated that references the raw SQL query that is attempted ``` Internal Server Error: /api/ipam/ip-addresses/ DataError at /api/ipam/ip-addresses/ invalid input syntax for type inet: "/32" LINE 1: ..."ipam_ipaddress" WHERE "ipam_ipaddress"."address" IN ('/32') ``` Example from the demo site ```Server Error There was a problem with your request. Please contact an administrator. The complete exception is provided below: <class 'django.db.utils.DataError'> invalid input syntax for type inet: "/32" LINE 1: ..."ipam_ipaddress" WHERE "ipam_ipaddress"."address" IN ('/32') ^ Python version: 3.8.10 NetBox version: 3.4.2 ```
adam added the type: bugstatus: accepted labels 2025-12-29 20:24:13 +01:00
adam closed this issue 2025-12-29 20:24:14 +01:00
adam commented 2025-12-29 20:24:15 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Jan 12, 2023):

Seems like the lookup's get_prep_lookup() method might be the culprit here:

8d9e151030/netbox/ipam/lookups.py (L106-L108)

Interestingly this seems to be a problem only when the offending value contains a slash, because it sidesteps calling HOST() when formulating the SQL query.

@jeremystretch commented on GitHub (Jan 12, 2023): Seems like the lookup's `get_prep_lookup()` method might be the culprit here: https://github.com/netbox-community/netbox/blob/8d9e151030c2708e1c103e8d0676d0621171dcbf/netbox/ipam/lookups.py#L106-L108 Interestingly this seems to be a problem only when the offending value contains a slash, because it sidesteps calling `HOST()` when formulating the SQL query.
adam commented 2025-12-29 20:24:15 +01:00
Author
Owner
Copy Link

@tyler-8 commented on GitHub (Jan 12, 2023):

My first thought would be to add checks for a valid IP (with or without mask) using netaddr, but not familiar enough with Lookup logic to say if that's even a good place for it.

Seems like any invalid IP address generates the error.

and while an error makes sense, I think the current output could be a little clearer/cleaner and probably not an HTTP 500 :D

@tyler-8 commented on GitHub (Jan 12, 2023): My first thought would be to add checks for a valid IP (with or without mask) using `netaddr`, but not familiar enough with `Lookup` logic to say if that's even a good place for it. Seems like any invalid IP address generates the error. - https://demo.netbox.dev/ipam/ip-addresses/?address=112222.11.11.22211/24 - https://demo.netbox.dev/ipam/ip-addresses/?address=192.0.2.1/2455 and while an error makes sense, I think the current output could be a little clearer/cleaner and probably not an HTTP 500 :D
adam commented 2025-12-29 20:24:15 +01:00
Author
Owner
Copy Link

@tyler-8 commented on GitHub (Jan 15, 2023):

Would a netaddr validation make sense in filtersets.py? The ipam filtersets already use netaddr and AddrFormatError checks so this seems more natural than going in the lookups.

Current:

d7c37d9dd6/netbox/ipam/filtersets.py (L588-L592)

New:

    def filter_address(self, queryset, name, value):
        try:
            valid_ip_obj = netaddr.IPNetwork(value)
            return queryset.filter(address__net_in=value)
        except (AddrFormatError, ValidationError, TypeError):
            return queryset.none()

TypeError was being raised passing just /32 to netaddr.IPNetwork so I'm handling that as well. Just a mask alone gets parsed as a list by django

image
@tyler-8 commented on GitHub (Jan 15, 2023): Would a `netaddr` validation make sense in `filtersets.py`? The `ipam` filtersets already use `netaddr` and `AddrFormatError` checks so this seems more natural than going in the `lookups`. Current: https://github.com/netbox-community/netbox/blob/d7c37d9dd6556654dcfa9ec940c3f1af9d4d3c6e/netbox/ipam/filtersets.py#L588-L592 New: ```python def filter_address(self, queryset, name, value): try: valid_ip_obj = netaddr.IPNetwork(value) return queryset.filter(address__net_in=value) except (AddrFormatError, ValidationError, TypeError): return queryset.none() ``` `TypeError` was being raised passing just `/32` to `netaddr.IPNetwork` so I'm handling that as well. Just a mask alone gets parsed as a list by `django` <img width="780" alt="image" src="https://user-images.githubusercontent.com/17618971/212568300-e46574d5-b102-44e1-b698-8396e6ae7361.png">
adam commented 2025-12-29 20:24:15 +01:00
Author
Owner
Copy Link

@rmanyari commented on GitHub (Feb 16, 2023):

Hi guys, I've been poking around Netbox on and off for the last two weeks or so and would love to contribute bug fixes as a way to get more active.

Agreed with Jeremy that the current bug is in the lookup logic, in particular in how we detect addresses with or without masks at:

(rhs_params value is ['/32'] (as evaluated by get_db_prep_lookup) which makes as_sql take the first branch in the if statement)

    def as_sql(self, qn, connection):
        lhs, lhs_params = self.process_lhs(qn, connection)
        rhs, rhs_params = self.process_rhs(qn, connection)
        with_mask, without_mask = [], []
        for address in rhs_params[0]:
            if '/' in address:
                with_mask.append(address)
            else:
                without_mask.append(address)

That being said, IMO input validation of this kind is better done at the API layer. I'm really happy to add something along the lines of what Tyler is proposing (leveraging the netaddr package for validation) and throw in a few unit tests.

Would this be OK as a first contribution from a rookie like me?

Thanks!

@rmanyari commented on GitHub (Feb 16, 2023): Hi guys, I've been poking around Netbox on and off for the last two weeks or so and would love to contribute bug fixes as a way to get more active. Agreed with Jeremy that the current bug is in the lookup logic, in particular in how we detect addresses with or without masks at: (`rhs_params` value is `['/32']` (as evaluated by `get_db_prep_lookup`) which makes `as_sql` take the first branch in the if statement) ``` def as_sql(self, qn, connection): lhs, lhs_params = self.process_lhs(qn, connection) rhs, rhs_params = self.process_rhs(qn, connection) with_mask, without_mask = [], [] for address in rhs_params[0]: if '/' in address: with_mask.append(address) else: without_mask.append(address) ``` That being said, IMO input validation of this kind is better done at the API layer. I'm really happy to add something along the lines of what Tyler is proposing (leveraging the `netaddr` package for validation) and throw in a few unit tests. Would this be OK as a first contribution from a rookie like me? Thanks!
adam commented 2025-12-29 20:24:16 +01:00
Author
Owner
Copy Link

@rmanyari commented on GitHub (Feb 16, 2023):

In addition of supporting IPNetwork we could also validate for IPAddress. Going over the netaddr docs there's a few options.

Here's what I propose which supports networks, valid addresses and also casts netmasks in the bit form to digit which is what's supported by the inet type in postgres.

netbox % git diff
diff --git a/netbox/ipam/filtersets.py b/netbox/ipam/filtersets.py
index c30064ff1..5dc1823cf 100644
--- a/netbox/ipam/filtersets.py
+++ b/netbox/ipam/filtersets.py
@@ -16,6 +16,7 @@ from virtualization.models import VirtualMachine, VMInterface
 from .choices import *
 from .models import *
 
+from rest_framework import serializers
 
 __all__ = (
     'AggregateFilterSet',
@@ -585,8 +586,19 @@ class IPAddressFilterSet(NetBoxModelFilterSet, TenancyFilterSet):
                 return queryset.none()
         return queryset.filter(q)
 
+    def parse_inet_address(self, value):
+        try:
+            addr = value[0]
+            if netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr):
+                return value
+            network = netaddr.IPNetwork(addr)
+            return [str(network)]
+        except:
+            raise serializers.ValidationError(f'Invalid address {addr}. It must be a valid IPv4 or IPv6 address or network')
+
     def filter_address(self, queryset, name, value):
         try:
+            value = self.parse_inet_address(value)
             return queryset.filter(address__net_in=value)
         except ValidationError:
             return queryset.none()

Below is the behavior I get on my local instance with those changes:

% curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=/32"                      
["Invalid address /32. It must be a valid IPv4 or IPv6 address or network"]%                                                                                                                                
% curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=32"                       
{"count":0,"next":null,"previous":null,"results":[]}%                                                                                                                                                       
% curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1"              
{"count":0,"next":null,"previous":null,"results":[]}%                                                                                                                                                       
% curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1/32"           
{"count":0,"next":null,"previous":null,"results":[]}%                                                                                                                                                       
% curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1/255.255.255.0"
{"count":0,"next":null,"previous":null,"results":[]}%

Let me know if this is inline with what you guys had in mind, as mentioned before, I'm happy to throw in a few unit tests and go through the standard CR process to get this fix in :)

@rmanyari commented on GitHub (Feb 16, 2023): In addition of supporting `IPNetwork` we could also validate for `IPAddress`. Going over the [netaddr docs there's a few options](https://netaddr.readthedocs.io/en/latest/api.html?highlight=validation#validation-functions). Here's what I propose which supports networks, valid addresses and also casts netmasks in the bit form to digit which is what's [supported by the inet type](https://www.postgresql.org/docs/current/datatype-net-types.html#DATATYPE-INET) in postgres. ``` netbox % git diff diff --git a/netbox/ipam/filtersets.py b/netbox/ipam/filtersets.py index c30064ff1..5dc1823cf 100644 --- a/netbox/ipam/filtersets.py +++ b/netbox/ipam/filtersets.py @@ -16,6 +16,7 @@ from virtualization.models import VirtualMachine, VMInterface from .choices import * from .models import * +from rest_framework import serializers __all__ = ( 'AggregateFilterSet', @@ -585,8 +586,19 @@ class IPAddressFilterSet(NetBoxModelFilterSet, TenancyFilterSet): return queryset.none() return queryset.filter(q) + def parse_inet_address(self, value): + try: + addr = value[0] + if netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr): + return value + network = netaddr.IPNetwork(addr) + return [str(network)] + except: + raise serializers.ValidationError(f'Invalid address {addr}. It must be a valid IPv4 or IPv6 address or network') + def filter_address(self, queryset, name, value): try: + value = self.parse_inet_address(value) return queryset.filter(address__net_in=value) except ValidationError: return queryset.none() ``` Below is the behavior I get on my local instance with those changes: ``` % curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=/32" ["Invalid address /32. It must be a valid IPv4 or IPv6 address or network"]% % curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=32" {"count":0,"next":null,"previous":null,"results":[]}% % curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1" {"count":0,"next":null,"previous":null,"results":[]}% % curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1/32" {"count":0,"next":null,"previous":null,"results":[]}% % curl -H "Authorization: Token $(./print-token.sh)" "http://localhost:${NETBOX_PORT}/api/ipam/ip-addresses/?address=192.168.0.1/255.255.255.0" {"count":0,"next":null,"previous":null,"results":[]}% ``` Let me know if this is inline with what you guys had in mind, as mentioned before, I'm happy to throw in a few unit tests and go through the standard CR process to get this fix in :)
adam commented 2025-12-29 20:24:16 +01:00
Author
Owner
Copy Link

@rmanyari commented on GitHub (Feb 21, 2023):

@jeremystretch @tyler-8 any thoughts on this?

@rmanyari commented on GitHub (Feb 21, 2023): @jeremystretch @tyler-8 any thoughts on this?
adam commented 2025-12-29 20:24:16 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Feb 28, 2023):

@rmanyari assigning to you per our Slack conversation.

@jeremystretch commented on GitHub (Feb 28, 2023): @rmanyari assigning to you per our Slack conversation.
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#7495
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?