From 61f6fe9d1c4cfbcc1cf8f00861ff06a4639a4c2e Mon Sep 17 00:00:00 2001
From: Alex Elder <elder@linaro.org>
Date: Fri, 1 Oct 2021 18:23:28 -0500
Subject: [PATCH] parser: be more restrictive when parsing numbers

When a number is parsed, the leading one or two characters are used
to indicate whether the number should be interpreted as hexadecimal,
octal or decimal.

But because the parser accepts any digits regardless of the base, it
allows things like 039 to be treated as an octal number, despite '9'
not being a valid digit.  The previous commit makes matters even
worse, allowing [a-fA-F] to be accepted for octal or decimal values.
Such errors are caught (but ignored) later when converting the
accepted string into a number in strtoull().

We are already looking at the first character or two to determine
the base, *after* scanning the number.  Instead, determine the base
when the first one or two characters are first input, and restrict
which characters are accepted in the number based on that.

As a consequence, strtoul() will examine all of the characters
comprising the number (whereas previously it would stop if it
encountered invalid character for the base).

Finally, accept either "0x" or "0X" to indicate hexadecimal.

This doesn't actually change behavior much, but as long as we're
checking every character in a number for validity we might as well
be restrictive.

Signed-off-by: Alex Elder <elder@linaro.org>
Message-Id: <20211001232338.769309-25-elder@linaro.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 parser.c              | 34 ++++++++++++++++++++++++++--------
 tests/bad_X.qmi       | 19 +++++++++++++++++++
 tests/bad_decimal.qmi | 19 +++++++++++++++++++
 tests/bad_hex.qmi     | 19 +++++++++++++++++++
 tests/bad_octal.qmi   | 19 +++++++++++++++++++
 5 files changed, 102 insertions(+), 8 deletions(-)
 create mode 100644 tests/bad_X.qmi
 create mode 100644 tests/bad_decimal.qmi
 create mode 100644 tests/bad_hex.qmi
 create mode 100644 tests/bad_octal.qmi

diff --git a/parser.c b/parser.c
index eed646b..fc40274 100644
--- a/parser.c
+++ b/parser.c
@@ -165,11 +165,18 @@ static bool skip(char ch)
 	return in_comment;
 }
 
+/* Used for parsing octal numbers */
+static int isodigit(int c)
+{
+	return isdigit(c) && c < '9';
+}
+
 static struct token yylex()
 {
 	struct symbol *sym;
 	struct token token = {};
 	unsigned long long num;
+	int (*isvalid)(int);
 	char buf[128];
 	char *p = buf;
 	int base;
@@ -214,20 +221,31 @@ static struct token yylex()
 
 		return token;
 	} else if (isdigit(ch)) {
+		/* Determine base and valid character set */
+		if (ch == '0') {
+			*p++ = ch;
+			ch = input();
+			if (ch == 'x' || ch == 'X') {
+				*p++ = ch;
+				ch = input();
+				isvalid = isxdigit;
+				base = 16;
+			} else {
+				isvalid = isodigit;
+				base = 8;
+			}
+		} else {
+			isvalid = isdigit;
+			base = 10;
+		}
+
 		do {
 			*p++ = ch;
 			ch = input();
-		} while (isxdigit(ch) || (p - buf == 1 && ch == 'x'));
+		} while (isvalid(ch));
 		unput(ch);
 		*p = '\0';
 
-		if (buf[0] == '0' && buf[1] == 'x')
-			base = 16;
-		else if (buf[0] == '0')
-			base = 8;
-		else
-			base = 10;
-
 		errno = 0;
 		num = strtoull(buf, NULL, base);
 		if (errno)
diff --git a/tests/bad_X.qmi b/tests/bad_X.qmi
new file mode 100644
index 0000000..d648835
--- /dev/null
+++ b/tests/bad_X.qmi
@@ -0,0 +1,19 @@
+package test;
+
+struct qmi_result {
+	u16 result;
+	u16 error;
+};
+
+request test_request {
+	# Previously "0X" was not allowed to indicate a hexadecimal value
+	required u8 test_number = 0X12;
+} = 0x23;
+
+response test_response {
+	required qmi_result r = 2;
+} = 043;
+
+indication test_indication {
+	optional u64 value = 0x99;
+} = 0x7;
diff --git a/tests/bad_decimal.qmi b/tests/bad_decimal.qmi
new file mode 100644
index 0000000..b9f8f2e
--- /dev/null
+++ b/tests/bad_decimal.qmi
@@ -0,0 +1,19 @@
+package test;
+
+struct qmi_result {
+	u16 result;
+	u16 error;
+};
+
+request test_request {
+	required u8 test_number = 0x12;
+} = 0x23;
+
+response test_response {
+	# Note that 'a' is not a valid decimal digit
+	required qmi_result r = 2a;
+} = 043;
+
+indication test_indication {
+	optional u64 value = 0x99;
+} = 0x7;
diff --git a/tests/bad_hex.qmi b/tests/bad_hex.qmi
new file mode 100644
index 0000000..1e6175c
--- /dev/null
+++ b/tests/bad_hex.qmi
@@ -0,0 +1,19 @@
+package test;
+
+struct qmi_result {
+	u16 result;
+	u16 error;
+};
+
+request test_request {
+	# Note that 'g' is not a valid hexadecimal digit
+	required u8 test_number = 0x1g;
+} = 0x23;
+
+response test_response {
+	required qmi_result r = 2;
+} = 043;
+
+indication test_indication {
+	optional u64 value = 0x99;
+} = 0x7;
diff --git a/tests/bad_octal.qmi b/tests/bad_octal.qmi
new file mode 100644
index 0000000..10c43be
--- /dev/null
+++ b/tests/bad_octal.qmi
@@ -0,0 +1,19 @@
+package test;
+
+struct qmi_result {
+	u16 result;
+	u16 error;
+};
+
+request test_request {
+	# Note that '8' is not a valid octal digit
+	optional test_struct foo = 028;
+} = 0x23;
+
+response test_response {
+	required qmi_result r = 2;
+} = 043;
+
+indication test_indication {
+	optional u64 value = 0x99;
+} = 0x7;