systemd: simplify parser and fix infinite loop

This commit simplifies the systemd parser logic, and it solves an
infinite loop when using a continuation line.

Closes: #24810

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Author: giuseppe

pkg/systemd/parser/split.go

@@ -55,8 +55,8 @@ func isValidUnicode(c uint32) bool {
 /* This is based on code from systemd (src/basic/escape.c), marked LGPL-2.1-or-later and is copyrighted by the systemd developers */
 
 func cUnescapeOne(p string, acceptNul bool) (int, rune, bool) {
-	var count = 1
-	var eightBit = false
+	count := 1
+	eightBit := false
 	var ret rune
 
 	// Unescapes C style. Returns the unescaped character in ret.
@@ -297,7 +297,7 @@ loop1:
 			}
 
 			if flags&(SplitCUnescape|SplitUnescapeSeparators) != 0 {
-				var r = -1
+				r := -1
 				var u rune
 
 				if flags&SplitCUnescape != 0 {

pkg/systemd/parser/unitfile.go

@@ -376,53 +376,36 @@ func (p *UnitFileParser) flushPendingComments(toComment bool) {
 	}
 }
 
-func nextLine(data string, afterPos int) (string, string) {
-	rest := data[afterPos:]
-	if i := strings.Index(rest, "\n"); i >= 0 {
-		return strings.TrimSpace(data[:i+afterPos]), data[i+afterPos+1:]
-	}
-	return data, ""
-}
-
-func trimSpacesFromLines(data string) string {
-	lines := strings.Split(data, "\n")
-	for i, line := range lines {
-		lines[i] = strings.TrimSpace(line)
-	}
-	return strings.Join(lines, "\n")
-}
-
 // Parse an already loaded unit file (in the form of a string)
 func (f *UnitFile) Parse(data string) error {
 	p := &UnitFileParser{
-		file:   f,
-		lineNr: 1,
+		file: f,
 	}
 
-	data = trimSpacesFromLines(data)
+	lines := strings.Split(strings.TrimSuffix(data, "\n"), "\n")
+	remaining := ""
 
-	for len(data) > 0 {
-		origdata := data
-		nLines := 1
-		var line string
-		line, data = nextLine(data, 0)
+	for lineNr, line := range lines {
+		p.lineNr = lineNr + 1
 
 		if !lineIsComment(line) {
-			// Handle multi-line continuations
-			// Note: This doesn't support comments in the middle of the continuation, which systemd does
-			if lineIsKeyValuePair(line) {
-				for len(data) > 0 && line[len(line)-1] == '\\' {
-					line, data = nextLine(origdata, len(line)+1)
-					nLines++
+			// check whether the line is a continuation of the previous line
+			if strings.HasSuffix(line, "\\") {
+				line = line[:len(line)-1]
+				if lineNr != len(lines)-1 {
+					remaining += line
+					continue
 				}
 			}
+			if remaining != "" {
+				line = remaining + line
+				remaining = ""
+			}
 		}
 
 		if err := p.parseLine(line); err != nil {
 			return err
 		}
-
-		p.lineNr += nLines
 	}
 
 	if p.currentGroup == nil {

pkg/systemd/parser/unitfile_test.go

@@ -2,6 +2,7 @@ package parser
 
 import (
 	"reflect"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -119,7 +120,9 @@ After=dbus.socket
 
 [Service]
 BusName=org.freedesktop.login1
-CapabilityBoundingSet=CAP_SYS_ADMIN CAP_MAC_ADMIN CAP_AUDIT_CONTROL CAP_CHOWN CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE CAP_FOWNER CAP_SYS_TTY_CONFIG CAP_LINUX_IMMUTABLE
+CapabilityBoundingSet=CAP_SYS_ADMIN CAP_MAC_ADMIN CAP_AUDIT_CONTROL CAP_CHOWN CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE # comment inside a continuation line \
+CAP_FOWNER \
+CAP_SYS_TTY_CONFIG CAP_LINUX_IMMUTABLE
 DeviceAllow=block-* r
 DeviceAllow=char-/dev/console rw
 DeviceAllow=char-drm rw
@@ -158,8 +161,8 @@ SystemCallFilter=@system-service
 
 # Increase the default a bit in order to allow many simultaneous logins since
 # we keep one fd open per session.
-LimitNOFILE=524288
-`
+LimitNOFILE=524288  \`
+
 const systemdnetworkdService = `#  SPDX-License-Identifier: LGPL-2.1-or-later
 #
 #  This file is part of systemd.
@@ -264,6 +267,14 @@ var sampleDropinPaths = map[string][]string{
 	sampleDropinTemplateInstance: sampleDropinTemplateInstancePaths,
 }
 
+func filterComments(input string) string {
+	// merge continuation lines
+	joined := strings.ReplaceAll(input, "\\\n", "")
+
+	// remove trailing newlines and backslashes
+	return strings.TrimSuffix(strings.TrimSuffix(joined, "\n"), "\\")
+}
+
 func TestRanges_Roundtrip(t *testing.T) {
 	for i := range samples {
 		sample := samples[i]
@@ -278,7 +289,7 @@ func TestRanges_Roundtrip(t *testing.T) {
 			panic(e)
 		}
 
-		assert.Equal(t, sample, asStr)
+		assert.Equal(t, filterComments(sample), filterComments(asStr))
 	}
 }
 
@@ -292,3 +303,16 @@ func TestUnitDropinPaths_Search(t *testing.T) {
 		assert.True(t, reflect.DeepEqual(expectedPaths, generatedPaths))
 	}
 }
+
+func FuzzParser(f *testing.F) {
+	for _, sample := range samples {
+		f.Add([]byte(sample))
+	}
+
+	f.Fuzz(func(t *testing.T, orig []byte) {
+		unitFile := NewUnitFile()
+		unitFile.Path = "foo/bar"
+		unitFile.Filename = "bar"
+		_ = unitFile.Parse(string(orig))
+	})
+}