Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions packages/bigquery-magics/bigquery_magics/graph_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@
import json
import socketserver
import threading
import urllib.parse
from typing import Any, Dict, List

from google.cloud import bigquery

from bigquery_magics import core

# The graph widget only ever reaches this server over the loopback interface.
# Requests carrying any other Host are cross-site (e.g. a DNS-rebinding page in
# the notebook user's browser) and are refused.
_LOOPBACK_HOSTS = frozenset({"localhost", "127.0.0.1", "::1"})


def execute_node_expansion(params, request):
return {"error": "Node expansion not yet implemented"}
Expand Down Expand Up @@ -354,6 +360,19 @@ class GraphServerHandler(http.server.SimpleHTTPRequestHandler):
def log_message(self, format, *args):
pass

def _host_is_loopback(self):
"""Return True if the request targets the loopback interface.

Origin cannot be used here because the notebook page and this server
live on different ports, so legitimate widget traffic is cross-origin.
The Host header, however, is always a loopback name for that traffic.
"""
host = self.headers.get("Host")
if not host:
return False
hostname = urllib.parse.urlsplit(f"//{host}").hostname
return hostname in _LOOPBACK_HOSTS
Comment on lines +373 to +374

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using urllib.parse.urlsplit on an arbitrary Host header can raise a ValueError (for example, if the Host header contains an IPv6 address without square brackets like ::1, or contains other invalid characters/control characters). To prevent unhandled exceptions and potential denial of service, wrap the parsing in a try...except ValueError block and return False as a graceful fallback.

Suggested change
hostname = urllib.parse.urlsplit(f"//{host}").hostname
return hostname in _LOOPBACK_HOSTS
try:
hostname = urllib.parse.urlsplit(f"//{host}").hostname
except ValueError:
return False
return hostname in _LOOPBACK_HOSTS
References
  1. Do not replace historical graceful fallback behaviors (such as returning False) with exceptions to maintain backwards compatibility.


def do_json_response(self, data):
self.send_response(200)
self.send_header("Access-Control-Allow-Origin", "*")
Expand Down Expand Up @@ -406,10 +425,16 @@ def handle_post_node_expansion(self):
)

def do_GET(self):
if not self._host_is_loopback():
self.send_error(403, "Forbidden")
return
assert self.path == GraphServer.endpoints["get_ping"]
self.handle_get_ping()

def do_POST(self):
if not self._host_is_loopback():
self.send_error(403, "Forbidden")
return
if self.path == GraphServer.endpoints["post_ping"]:
self.handle_post_ping()
elif self.path == GraphServer.endpoints["post_node_expansion"]:
Expand Down
30 changes: 30 additions & 0 deletions packages/bigquery-magics/tests/unit/test_graph_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,3 +747,33 @@ def test_convert_schema_shared_label():
labels = {label["name"]: label for label in result["labels"]}
assert "Person" in labels
assert set(labels["Person"]["propertyDeclarationNames"]) == {"id", "name"}


class TestGraphServerHostHeader(unittest.TestCase):
def setUp(self):
self.server = graph_server.GraphServer()
self.server_thread = self.server.init()

def tearDown(self):
self.server.stop_server()
self.server_thread.join()

def _route(self):
return self.server.build_route(graph_server.GraphServer.endpoints["get_ping"])

def test_loopback_host_allowed(self):
response = requests.get(self._route())
self.assertEqual(response.status_code, 200)

def test_non_loopback_host_rejected(self):
# A DNS-rebinding page reaches the loopback socket but carries the
# attacker's hostname in the Host header.
response = requests.get(self._route(), headers={"Host": "attacker.example"})
self.assertEqual(response.status_code, 403)

def test_non_loopback_host_rejected_post(self):
route = self.server.build_route(graph_server.GraphServer.endpoints["post_ping"])
response = requests.post(
route, json={"data": "ping"}, headers={"Host": "evil.example:1234"}
)
self.assertEqual(response.status_code, 403)
Loading