From a4c482f66c22a6344a2bb25a71bcc70cd5ab0508 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Mon, 2 May 2022 16:38:30 +0200
Subject: [PATCH] Config: prevent config crashes when an attribute already
 exists, while here also make sure we report the error as it will now silently
 fail. SimpleXMLElement's addAttribute() is only valida when the it doesn't
 already exist.

Strangly enough, this doesn't seem to happen very often, but during ha sync we are able to crash without notice.
---
 .../mvc/app/library/OPNsense/Core/Config.php    | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
index 68426b437..ddf7cf142 100644
--- a/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
+++ b/src/opnsense/mvc/app/library/OPNsense/Core/Config.php
@@ -193,7 +193,10 @@ class Config extends Singleton
             $node = $this->simplexml;
             // invalidate object on warnings/errors (prevent save from happening)
             set_error_handler(
-                function () {
+                function ($errno, $errstr, $errfile, $errline) {
+                    syslog(LOG_ERR, sprintf(
+                        "Config serialize error [%d] %s @ %s : %s", $errno, $errstr, $errfile, $errline
+                    ));
                     $this->statusIsValid = false;
                 }
             );
@@ -209,7 +212,11 @@ class Config extends Singleton
             if ($itemKey === '@attributes') {
                 // copy xml attributes
                 foreach ($itemValue as $attrKey => $attrValue) {
-                    $node->addAttribute($attrKey, $attrValue);
+                    if (isset($node->attributes()[$attrKey])) {
+                        $node->attributes()->$attrKey = $attrValue;
+                    } else {
+                        $node->addAttribute($attrKey, $attrValue);
+                    }
                 }
                 continue;
             } elseif (strstr($itemKey, '@attributes') !== false) {
@@ -217,7 +224,11 @@ class Config extends Singleton
                 if (count($node->$origname)) {
                     // copy xml attributes
                     foreach ($itemValue as $attrKey => $attrValue) {
-                        $node->$origname->addAttribute($attrKey, $attrValue);
+                        if (isset($node->$origname->attributes()[$attrKey])) {
+                            $node->$origname->attributes()->$attrKey = $attrValue;
+                        } else {
+                            $node->$origname->addAttribute($attrKey, $attrValue);
+                        }
                     }
                 }
                 continue;