From 91ebb21622ab09610ecef04274a4025830e0c585 Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Wed, 3 Dec 2025 12:31:20 +0100
Subject: [PATCH] net/upnp: fix escaping in the file for #5005

* remove pconfig escaping as it is already escaped globally
* change htmlspecialchars() to html_safe()
* protect some spots with html_safe() (being legacy code this is an uphill battle)
* some more style tweaks for readability
---
 net/upnp/src/www/services_upnp.php | 40 ++++++++++++++----------------
 1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/net/upnp/src/www/services_upnp.php b/net/upnp/src/www/services_upnp.php
index e4919ab2c..ec5402042 100644
--- a/net/upnp/src/www/services_upnp.php
+++ b/net/upnp/src/www/services_upnp.php
@@ -263,13 +263,11 @@ include("head.inc");
                       <td><a id="help_for_ext_iface" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("External interface");?></td>
                       <td>
                        <select class="selectpicker" name="ext_iface">
-<?php
-                        foreach (get_configured_interface_with_descr() as $iface => $ifacename):?>
-                          <option value="<?=$iface;?>" <?=$pconfig['ext_iface'] == $iface ? "selected=\"selected\"" : "";?>>
-                            <?=htmlspecialchars($ifacename);?>
+<?php foreach (get_configured_interface_with_descr() as $iface => $ifacename): ?>
+                          <option value="<?= html_safe($iface) ?>" <?= $pconfig['ext_iface'] == $iface ? 'selected="selected"' : '' ?>>
+                            <?= html_safe($ifacename) ?>
                           </option>
-<?php
-                        endforeach;?>
+<?php endforeach ?>
                        </select>
                        <div class="hidden" data-for="help_for_ext_iface">
                          <?=gettext("The WAN network interface containing the default gateway.");?>
@@ -280,16 +278,14 @@ include("head.inc");
                       <td><a id="help_for_iface_array" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Internal interfaces");?></td>
                       <td>
                        <select class="selectpicker" name="iface_array[]" multiple="multiple">
-                         <option value="lo0" <?=!empty($pconfig['iface_array']) && in_array('lo0', $pconfig['iface_array']) ? "selected=\"selected\"" : "";?>>
-                           <?=gettext("Localhost");?>
+                         <option value="lo0" <?=!empty($pconfig['iface_array']) && in_array('lo0', $pconfig['iface_array']) ? 'selected="selected"' : '' ?>>
+                           <?= html_safe(gettext('Localhost')) ?>
                          </option>
-<?php
-                        foreach (get_configured_interface_with_descr() as $iface => $ifacename):?>
-                          <option value="<?=$iface;?>" <?=!empty($pconfig['iface_array']) && in_array($iface, $pconfig['iface_array']) ? "selected=\"selected\"" : "";?>>
-                            <?=htmlspecialchars($ifacename);?>
+<?php foreach (get_configured_interface_with_descr() as $iface => $ifacename): ?>
+                          <option value="<?= html_safe($iface) ?>" <?= in_array($iface, $pconfig['iface_array'] ?? []) ? 'selected="selected"' : '' ?>>
+                            <?= html_safe($ifacename) ?>
                           </option>
-<?php
-                        endforeach;?>
+<?php endforeach ?>
                        </select>
                        <div class="hidden" data-for="help_for_iface_array">
                          <?=gettext("Select one or more internal network interfaces, such as LAN, where clients reside.");?>
@@ -315,7 +311,7 @@ include("head.inc");
                     <tr>
                       <td><a id="help_for_stun_host" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?= gettext('STUN server') ?></td>
                       <td>
-                        <input name="stun_host" type="text" value="<?= !empty($pconfig['stun_host']) ? $pconfig['stun_host'] : '' ?>" />
+                        <input name="stun_host" type="text" value="<?= $pconfig['stun_host'] ?? '' ?>" />
                         <div class="hidden" data-for="help_for_stun_host">
                           <?= gettext('Allow use of unrestricted endpoint-independent (1:1) CGNATs and detect the public IPv4 using e.g. "stun.3cx.com" or "stun.counterpath.com".') ?>
                         </div>
@@ -324,7 +320,7 @@ include("head.inc");
                     <tr>
                       <td><i class="fa fa-info-circle text-muted"></i> <?= gettext('STUN port') ?></td>
                       <td>
-                        <input name="stun_port" type="text" placeholder="3478" value="<?= !empty($pconfig['stun_port']) ? $pconfig['stun_port'] : ''  ?>" />
+                        <input name="stun_port" type="text" placeholder="3478" value="<?= $pconfig['stun_port'] ?? ''  ?>" />
                       </td>
                     </tr>
                     <tr>
@@ -400,8 +396,8 @@ include("head.inc");
                     <td><i class="fa fa-info-circle text-muted"></i> <?= gettext('UPnP IGD compatibility mode') ?></td>
                     <td>
                       <select name="upnp_igd_compat">
-                        <option value="igdv1" <?= $pconfig['upnp_igd_compat'] == 'igdv1' ? "selected=\"selected\"" : ""; ?> ><?= gettext("IGDv1 (IPv4 only)"); ?></option>
-                        <option value="igdv2" <?= $pconfig['upnp_igd_compat'] == 'igdv2' ? "selected=\"selected\"" : ""; ?> ><?= gettext("IGDv2 (with workarounds)"); ?></option>
+                        <option value="igdv1" <?= ($pconfig['upnp_igd_compat'] ?? '') == 'igdv1' ? 'selected="selected"' : '' ?> ><?= gettext('IGDv1 (IPv4 only)') ?></option>
+                        <option value="igdv2" <?= ($pconfig['upnp_igd_compat'] ?? '') == 'igdv2' ? 'selected="selected"' : '' ?> ><?= gettext('IGDv2 (with workarounds)') ?></option>
                       </select>
                     </td>
                   </tr>
@@ -426,7 +422,7 @@ include("head.inc");
                     <tr>
                       <td><i class="fa fa-info-circle text-muted"></i> <?= gettext('Router/friendly name') ?></td>
                       <td>
-                        <input name="friendly_name" type="text" placeholder="OPNsense UPnP IGD &amp; PCP" value="<?= !empty($pconfig['friendly_name']) ? htmlspecialchars($pconfig['friendly_name']) : '' ?>" />
+                        <input name="friendly_name" type="text" placeholder="OPNsense UPnP IGD &amp; PCP" value="<?= $pconfig['friendly_name'] ?? '' ?>" />
                       </td>
                     </tr>
                   </tbody>
@@ -456,7 +452,7 @@ include("head.inc");
                     <tr>
                       <td><a id="help_for_num_permuser" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Number of entries");?></td>
                       <td>
-                        <input name="num_permuser" type="text" placeholder="8" value="<?= html_safe($pconfig['num_permuser']) ?>" />
+                        <input name="num_permuser" type="text" placeholder="8" value="<?= $pconfig['num_permuser'] ?>" />
                         <div class="hidden" data-for="help_for_num_permuser">
                           <?=gettext("Number of ACL entries to configure.");?>
                         </div>
@@ -470,7 +466,7 @@ include("head.inc");
                       <td style="width:22%"><i class="fa fa-info-circle text-muted"></i> <?=gettext('ACL entry') . ' ' . $i ?></td>
 <?php endif ?>
                       <td style="width:78%">
-                        <input name="<?= html_safe($permuser) ?>" type="text" value="<?= isset($pconfig[$permuser]) ? $pconfig[$permuser] : '' ?>" />
+                        <input name="<?= html_safe($permuser) ?>" type="text" value="<?= $pconfig[$permuser] ?? '' ?>" />
 <?php if ($i == 1): ?>
                         <div class="hidden" data-for="help_for_permuser">
                           <?=gettext("Syntax: (allow or deny) (ext port or range) (int IP or IP/netmask) (int port or range)");?><br/>
@@ -494,7 +490,7 @@ include("head.inc");
                     <tr>
                      <td style="width:22%; vertical-align:top">&nbsp;</td>
                      <td style="width:78%">
-                       <input name="Submit" type="submit" class="btn btn-primary" value="<?=gettext("Save");?>" />
+                       <input name="Submit" type="submit" class="btn btn-primary" value="<?= html_safe(gettext('Save')) ?>" />
                      </td>
                     </tr>
                   </tbody>