From e8dcccf1a8863198cb03fddbb87454d5e5334675 Mon Sep 17 00:00:00 2001
From: Josef Schlehofer <pepe.schlehofer@gmail.com>
Date: Mon, 10 Aug 2026 08:45:12 +0200
Subject: [PATCH] fix: validate telemetry measurements

Normalize invalid measurements to NULL so malformed telemetry cannot break
result rendering or aggregate statistics.
---
 results/index.php        |  9 ++++++++
 results/telemetry_db.php | 46 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/results/index.php b/results/index.php
index 4559315..80eb0ce 100755
--- a/results/index.php
+++ b/results/index.php
@@ -31,6 +31,15 @@ function tryFont($name)
  */
 function format($d)
 {
+    // Rows written before the telemetry endpoint validated its input can hold
+    // anything, and the columns are declared `text` in every schema. An empty
+    // string reaching number_format() is a TypeError on PHP 8, which killed the
+    // whole image before its Content-Type header was even sent, so a result
+    // with a blank measurement answered 500 rather than drawing a zero.
+    if (!is_numeric($d)) {
+        $d = 0;
+    }
+
     if ($d < 10) {
         return number_format($d, 2, '.', '');
     }
diff --git a/results/telemetry_db.php b/results/telemetry_db.php
index 77b05ae..d583969 100755
--- a/results/telemetry_db.php
+++ b/results/telemetry_db.php
@@ -186,11 +186,57 @@ function isObfuscationEnabled()
         && true === $enable_id_obfuscation;
 }
 
+/**
+ * Normalises a measurement into a number, or null when it is not one.
+ *
+ * The four measurement columns are declared `text` in every schema shipped
+ * here, and nothing validated what went into them, so a client could store any
+ * string it liked. That has consequences beyond statistics: `format()` in
+ * index.php hands the value to `number_format()`, which on PHP 8 raises a
+ * TypeError for a string, so a single empty `dl=` turned that result's share
+ * image into a 500.
+ *
+ * Storing null instead keeps the row — the test still happened, and the IP,
+ * user agent and timestamp are still worth having — while leaving the value out
+ * of aggregates, which skip nulls. `COUNT(*) - COUNT(dl)` then reports how many
+ * submissions arrived malformed, so nothing is hidden by accepting them.
+ *
+ * No upper bound is imposed: links keep getting faster, and a cap would
+ * silently discard the fastest real results.
+ *
+ * @param mixed $value
+ *
+ * @return float|null
+ */
+function normalizeMeasurement($value)
+{
+    if (is_array($value) || is_object($value) || is_bool($value) || null === $value) {
+        return null;
+    }
+
+    $value = trim((string) $value);
+    if ('' === $value || !is_numeric($value)) {
+        return null;
+    }
+
+    $number = (float) $value;
+    if (!is_finite($number) || $number < 0) {
+        return null;
+    }
+
+    return $number;
+}
+
 /**
  * @return string|false returns the id of the inserted column or false on error if returnErrorMessage is false or a error message if returnErrorMessage is true
  */
 function insertSpeedtestUser($ip, $ispinfo, $extra, $ua, $lang, $dl, $ul, $ping, $jitter, $log, $returnExceptionOnError = false)
 {
+    $dl = normalizeMeasurement($dl);
+    $ul = normalizeMeasurement($ul);
+    $ping = normalizeMeasurement($ping);
+    $jitter = normalizeMeasurement($jitter);
+
     $pdo = getPdo();
     if (!($pdo instanceof PDO)) {
 		if($returnExceptionOnError){