From 0dd857bf03e50af337f45f18582d05573207b3dd Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Wed, 8 Jan 2025 11:54:20 +0100
Subject: [PATCH] system: refactor route creation for exec_safe() use

While here ignore all return values consistently.  Most of them are
ignored, some of them already on the add case.  Makes little sense
to report errors on a fraction of operations and adding the same route
will unfortunately yield an error anyway so it's not helpful in the
bulk of cases either.
---
 src/etc/inc/system.inc | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/etc/inc/system.inc b/src/etc/inc/system.inc
index 577c322ee..62d3a142e 100644
--- a/src/etc/inc/system.inc
+++ b/src/etc/inc/system.inc
@@ -735,46 +735,52 @@ function system_routing_configure($verbose = false, $interface_map = null, $moni
                 continue;
             }
 
-            $interfacegw = $gateway['if'];
-            $gatewayip = $gateway['gateway'] ?? '';
-            $fargw = !empty($gateway['fargw']) && $gateway['ipprotocol'] != 'inet6';
-            $blackhole = '';
+            $cmd = [exec_safe('-%s', $ipproto)];
 
             switch ($rtent['gateway']) {
                 case 'Null4':
                 case 'Null6':
-                    $blackhole = '-blackhole';
+                    $cmd[] = '-blackhole';
                     break;
                 default:
                     break;
             }
 
-            /* XXX some day we might want to convert to safer mwexecf() */
-            $cmd = " -{$ipproto} {$blackhole} " . escapeshellarg($rtent['network']) . " ";
+            $cmd[] = exec_safe('%s', $rtent['network']);
+
             if (!empty($rtent['disabled'])) {
-                mwexec("/sbin/route delete {$cmd}", true);
+                mwexec('/sbin/route delete ' . join(' ', $cmd), true);
                 continue;
             }
 
+            $gatewayip = $gateway['gateway'] ?? '';
+            $interfacegw = $gateway['if'];
+
             if (is_ipaddr($gatewayip)) {
-                mwexec("/sbin/route delete {$cmd}", true);
+                mwexec('/sbin/route delete ' . join(' ', $cmd), true);
 
                 if (ipproto == 'inet' && is_ipaddrv6($gatewayip)) {
-                    /* RFC 5549: gateway protocol differs */
-                    $cmd .= ' -inet6 ';
+                    $cmd[] = '-inet6'; /* RFC 5549: gateway protocol differs */
                 }
 
-                if ($fargw) {
+                if (!empty($gateway['fargw']) && $gateway['ipprotocol'] != 'inet6') {
                     mwexecf('/sbin/route delete -%s %s -interface %s ', [$ipproto, $gatewayip, $interfacegw], true);
-                    mwexecf('/sbin/route add -%s %s -interface %s', [$ipproto, $gatewayip, $interfacegw]);
+                    mwexecf('/sbin/route add -%s %s -interface %s', [$ipproto, $gatewayip, $interfacegw], true);
                 } elseif (is_linklocal($gatewayip) && strpos($gatewayip, '%') === false) {
                     $gatewayip .= "%{$interfacegw}";
                 }
-                mwexec("/sbin/route add" . $cmd . escapeshellarg($gatewayip));
+
+                $cmd = exec_safe('%s', $gatewayip);
+
+                mwexec('/sbin/route add ' . join(' ', $cmd), true);
             } elseif (!empty($interfacegw)) {
-                mwexec("/sbin/route delete" . $cmd . "-interface " . escapeshellarg($interfacegw), true);
-                mwexec("/sbin/route add" . $cmd . "-interface " . escapeshellarg($interfacegw));
+                $cmd[] = '-interface';
+                $cmd[] = exec_safe('%s', $interfacegw);
+
+                mwexec('/sbin/route delete ' . join(' ', $cmd), true);
             }
+
+            mwexec('/sbin/route add ' . join(' ', $cmd), true);
         }
     }