[3/3] initscripts: readhash: Fix handling = signs

Message ID 20241206164217.3840140-3-michael.tremer@ipfire.org
State Staged
Commit ff4ff2cfe0c8565a431bf499708dcb6e5c2fb3dc
Headers
Series [1/3] tests: Fix path to bash |

Commit Message

Michael Tremer Dec. 6, 2024, 4:42 p.m. UTC
  The function expected that a line only contains exactly one equals sign
(=) which is not fit for purpose. In the WireGuard code we hold key
material that is encoded in base64 and therefore contains padding that
uses =.

This patch fixes that we expect exactly one equals sign immediately
after the key and we will then accept more = in the value - which was
already permitted.

Furthermore, this patch fixes the splitting if the key and value at the
first =.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---
 src/initscripts/system/functions                     | 12 +++---------
 .../system/functions/data/2_output_stderr            |  8 ++++----
 2 files changed, 7 insertions(+), 13 deletions(-)
  

Comments

Jonatan Schlag Dec. 19, 2024, 6:16 p.m. UTC | #1
Hi,

as this is already staged, I do not reply to all emails, but the last
one: This looks all good. I do not unterstand the purpose of the second
patch, but I guess there is a reason.

Jonatan

Am Freitag, dem 06.12.2024 um 16:42 +0000 schrieb Michael Tremer:
> The function expected that a line only contains exactly one equals
> sign
> (=) which is not fit for purpose. In the WireGuard code we hold key
> material that is encoded in base64 and therefore contains padding
> that
> uses =.
> 
> This patch fixes that we expect exactly one equals sign immediately
> after the key and we will then accept more = in the value - which was
> already permitted.
> 
> Furthermore, this patch fixes the splitting if the key and value at
> the
> first =.
> 
> Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
> ---
>  src/initscripts/system/functions                     | 12 +++-------
> --
>  .../system/functions/data/2_output_stderr            |  8 ++++----
>  2 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/src/initscripts/system/functions
> b/src/initscripts/system/functions
> index 094e35495..e486cc085 100644
> --- a/src/initscripts/system/functions
> +++ b/src/initscripts/system/functions
> @@ -909,21 +909,15 @@ readhash() {
>                         continue
>                 fi
>  
> -               # Skip lines without a =
> -               if ! [[ ${line} =~ [^=]*=[^=]*$ ]]; then
> +               # Check for a valid key followed by =
> +               if ! [[ ${line} =~ ^[A-Za-z_][A-Za-z0-9_]*= ]]; then
>                         echo "Invalid line '${line}'" >&2
>                         continue
>                 fi
>  
> -               local key="${line%=*}"
> +               local key="${line%%=*}"
>                 local val="${line#*=}"
>  
> -               # Skip lines with an invalid key
> -               if ! [[ ${key} =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
> -                       echo "Invalid key '${key}'" >&2
> -                       continue
> -               fi
> -
>                 # Skip lines with invalid values
>                 if ! [[ ${val} =~ ^[\'][\ A-Za-z0-9=/,.:%_@#+-]*[\']$
> ]] && ! [[ ${val} =~ ^[A-Za-z0-9=/,.:%_@#+-]*$ ]]; then
>                         echo "Invalid value '${val}' for key
> '${key}'" >&2
> diff --git
> a/tests/src/initscripts/system/functions/data/2_output_stderr
> b/tests/src/initscripts/system/functions/data/2_output_stderr
> index 82f035e26..7f4c5a944 100644
> --- a/tests/src/initscripts/system/functions/data/2_output_stderr
> +++ b/tests/src/initscripts/system/functions/data/2_output_stderr
> @@ -2,8 +2,8 @@ Invalid value '?3' for key 'CONFIG_TYPE'
>  Invalid value 'gree!n0' for key 'GREEN_DEV'
>  Invalid value '00:c0:08:8a  :a0:47' for key 'GREEN_MACADDR'
>  Invalid value '"r8175"' for key 'GREEN_DRIVER'
> -Invalid key '-RED_DEV'
> -Invalid key 'RE??D_MACADDR'
> -Invalid key 'RED&&_DRIVER'
> -Invalid key '0BLUE_DEV'
> +Invalid line '-RED_DEV=red0'
> +Invalid line 'RE??D_MACADDR=00:c0:08:8a:a0:56'
> +Invalid line 'RED&&_DRIVER=r8283'
> +Invalid line '0BLUE_DEV='blue0 net0''
>  Invalid line 'Line_without_a_equal_sign_is_also_invalid'
  

Patch

diff --git a/src/initscripts/system/functions b/src/initscripts/system/functions
index 094e35495..e486cc085 100644
--- a/src/initscripts/system/functions
+++ b/src/initscripts/system/functions
@@ -909,21 +909,15 @@  readhash() {
 			continue
 		fi
 
-		# Skip lines without a =
-		if ! [[ ${line} =~ [^=]*=[^=]*$ ]]; then
+		# Check for a valid key followed by =
+		if ! [[ ${line} =~ ^[A-Za-z_][A-Za-z0-9_]*= ]]; then
 			echo "Invalid line '${line}'" >&2
 			continue
 		fi
 
-		local key="${line%=*}"
+		local key="${line%%=*}"
 		local val="${line#*=}"
 
-		# Skip lines with an invalid key
-		if ! [[ ${key} =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
-			echo "Invalid key '${key}'" >&2
-			continue
-		fi
-
 		# Skip lines with invalid values
 		if ! [[ ${val} =~ ^[\'][\ A-Za-z0-9=/,.:%_@#+-]*[\']$ ]] && ! [[ ${val} =~ ^[A-Za-z0-9=/,.:%_@#+-]*$ ]]; then
 			echo "Invalid value '${val}' for key '${key}'" >&2
diff --git a/tests/src/initscripts/system/functions/data/2_output_stderr b/tests/src/initscripts/system/functions/data/2_output_stderr
index 82f035e26..7f4c5a944 100644
--- a/tests/src/initscripts/system/functions/data/2_output_stderr
+++ b/tests/src/initscripts/system/functions/data/2_output_stderr
@@ -2,8 +2,8 @@  Invalid value '?3' for key 'CONFIG_TYPE'
 Invalid value 'gree!n0' for key 'GREEN_DEV'
 Invalid value '00:c0:08:8a  :a0:47' for key 'GREEN_MACADDR'
 Invalid value '"r8175"' for key 'GREEN_DRIVER'
-Invalid key '-RED_DEV'
-Invalid key 'RE??D_MACADDR'
-Invalid key 'RED&&_DRIVER'
-Invalid key '0BLUE_DEV'
+Invalid line '-RED_DEV=red0'
+Invalid line 'RE??D_MACADDR=00:c0:08:8a:a0:56'
+Invalid line 'RED&&_DRIVER=r8283'
+Invalid line '0BLUE_DEV='blue0 net0''
 Invalid line 'Line_without_a_equal_sign_is_also_invalid'