[PATCH bpf-next v5 3/3] bpftool: Fix bypass of the batch line length check by comments

From: Yuan Chen <[email protected]>

do_batch() strips trailing comments by truncating the line at '#'
before checking whether fgets() filled the buffer. If a batch line
longer than the buffer contains a '#' within the first
sizeof(buf) - 1 bytes, the truncation makes strlen(buf) smaller and the
line-length check is bypassed. The unread remainder of the line then
stays in the file stream and is parsed and executed as a separate
command on the next loop iteration.

Continuation lines handled below are affected the same way: an overlong
continuation line containing '#' bypasses the "command is too
long" check, and its unread remainder is executed as a separate command.

Detect the truncated read before stripping the comment, using memchr()
to look for a newline (instead of strlen(), which is also fooled by an
embedded NUL byte) and feof() to tell an overlong line apart from a
final line without a trailing newline. A line that fills the buffer
exactly (the byte after the read is a newline) is not treated as
truncated, so valid maximal-length lines are no longer rejected. Use the
result for the line-length checks, so overlong lines are rejected
regardless of comments or NUL bytes.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Signed-off-by: Yuan Chen <[email protected]>
---
 tools/bpf/bpftool/main.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 0bbb2e198450..337fbf1ec596 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -367,11 +367,27 @@ static int do_batch(int argc, char **argv)
 	if (json_output)
 		jsonw_start_array(json_wtr);
 	while (fgets(buf, sizeof(buf), fp)) {
+		bool truncated = !memchr(buf, '\n', sizeof(buf) - 1) && !feof(fp);
+
+		if (truncated) {
+			/*
+			 * fgets() filled the buffer. If the very next byte is
+			 * a newline, the line fits the buffer exactly and is
+			 * not truncated.
+			 */
+			int c = fgetc(fp);
+
+			if (c == '\n')
+				truncated = false;
+			else if (c != EOF)
+				ungetc(c, fp);
+		}
+
 		cp = strchr(buf, '#');
 		if (cp)
 			*cp = '\0';
 
-		if (strlen(buf) == sizeof(buf) - 1) {
+		if (truncated) {
 			line_too_long = true;
 			break;
 		}
@@ -380,6 +396,8 @@ static int do_batch(int argc, char **argv)
 		 * with '\' in the batch file).
 		 */
 		while ((cp = strstr(buf, "\\\n")) != NULL) {
+			bool cont_truncated;
+
 			if (!fgets(contline, sizeof(contline), fp) ||
 			    strlen(contline) == 0) {
 				p_err("missing continuation line on command %u",
@@ -388,11 +406,15 @@ static int do_batch(int argc, char **argv)
 				goto err_close;
 			}
 
+			cont_truncated = !memchr(contline, '\n', sizeof(contline) - 1) &&
+					 !feof(fp);
+
 			cp = strchr(contline, '#');
 			if (cp)
 				*cp = '\0';
 
-			if (strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
+			if (cont_truncated ||
+			    strlen(buf) + strlen(contline) + 1 > sizeof(buf)) {
 				p_err("command %u is too long", lines);
 				err = -1;
 				goto err_close;
-- 
2.54.0