[PATCH nft,v2] src: file descriptor leak in include_file()

[Pablo Neira Ayuso <pablo@netfilter.org>]

File that contains the ruleset is never closed, track open files through
the nft_ctx object and close them accordingly.

Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: skip a few redundant scanner parameters in functions

 include/nftables.h |  3 +++
 include/parser.h   |  6 +++---
 src/libnftables.c  |  6 +++---
 src/scanner.l      | 42 +++++++++++++++++++++++-------------------
 4 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/include/nftables.h b/include/nftables.h
index 5c0292615b3f..b17a16a4adef 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -86,6 +86,8 @@ struct nft_cache {
 struct mnl_socket;
 struct parser_state;
 
+#define MAX_INCLUDE_DEPTH	16
+
 struct nft_ctx {
 	struct mnl_socket	*nf_sock;
 	char			**include_paths;
@@ -99,6 +101,7 @@ struct nft_ctx {
 	struct parser_state	*state;
 	void			*scanner;
 	void			*json_root;
+	FILE			*f[MAX_INCLUDE_DEPTH];
 };
 
 enum nftables_exit_codes {
diff --git a/include/parser.h b/include/parser.h
index ea41ca038a02..8e57899eb1a3 100644
--- a/include/parser.h
+++ b/include/parser.h
@@ -3,8 +3,8 @@
 
 #include <list.h>
 #include <rule.h> // FIXME
+#include <nftables.h>
 
-#define MAX_INCLUDE_DEPTH		16
 #define TABSIZE				8
 
 #define YYLTYPE				struct location
@@ -36,9 +36,9 @@ extern void parser_init(struct nft_ctx *nft, struct parser_state *state,
 extern int nft_parse(struct nft_ctx *ctx, void *, struct parser_state *state);
 
 extern void *scanner_init(struct parser_state *state);
-extern void scanner_destroy(void *scanner);
+extern void scanner_destroy(struct nft_ctx *nft);
 
-extern int scanner_read_file(void *scanner, const char *filename,
+extern int scanner_read_file(struct nft_ctx *nft, const char *filename,
 			     const struct location *loc);
 extern int scanner_include_file(struct nft_ctx *ctx, void *scanner,
 				const char *filename,
diff --git a/src/libnftables.c b/src/libnftables.c
index 2271d270fd57..199dbc97b801 100644
--- a/src/libnftables.c
+++ b/src/libnftables.c
@@ -364,7 +364,7 @@ static int nft_parse_bison_filename(struct nft_ctx *nft, const char *filename,
 
 	parser_init(nft, nft->state, msgs, cmds);
 	nft->scanner = scanner_init(nft->state);
-	if (scanner_read_file(nft->scanner, filename, &internal_location) < 0)
+	if (scanner_read_file(nft, filename, &internal_location) < 0)
 		return -1;
 
 	ret = nft_parse(nft, nft->scanner, nft->state);
@@ -405,7 +405,7 @@ err:
 	}
 	iface_cache_release();
 	if (nft->scanner) {
-		scanner_destroy(nft->scanner);
+		scanner_destroy(nft);
 		nft->scanner = NULL;
 	}
 	free(nlbuf);
@@ -449,7 +449,7 @@ err:
 	}
 	iface_cache_release();
 	if (nft->scanner) {
-		scanner_destroy(nft->scanner);
+		scanner_destroy(nft);
 		nft->scanner = NULL;
 	}
 
diff --git a/src/scanner.l b/src/scanner.l
index 6f83aa1198cc..558bf9209853 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -659,19 +659,17 @@ static void scanner_pop_buffer(yyscan_t scanner)
 	state->indesc = &state->indescs[--state->indesc_idx - 1];
 }
 
-static struct error_record *scanner_push_file(void *scanner, const char *filename,
-					      FILE *f, const struct location *loc)
+static struct error_record *scanner_push_file(struct nft_ctx *nft, void *scanner,
+					      const char *filename, const struct location *loc)
 {
 	struct parser_state *state = yyget_extra(scanner);
 	YY_BUFFER_STATE b;
 
-	if (state->indesc_idx == MAX_INCLUDE_DEPTH) {
-		fclose(f);
+	if (state->indesc_idx == MAX_INCLUDE_DEPTH)
 		return error(loc, "Include nested too deeply, max %u levels",
 			     MAX_INCLUDE_DEPTH);
-	}
 
-	b = yy_create_buffer(f, YY_BUF_SIZE, scanner);
+	b = yy_create_buffer(nft->f[state->indesc_idx], YY_BUF_SIZE, scanner);
 	yypush_buffer_state(b, scanner);
 
 	state->indesc = &state->indescs[state->indesc_idx++];
@@ -683,8 +681,8 @@ static struct error_record *scanner_push_file(void *scanner, const char *filenam
 	return NULL;
 }
 
-static int include_file(void *scanner, const char *filename,
-			const struct location *loc)
+static int include_file(struct nft_ctx *nft, void *scanner,
+			const char *filename, const struct location *loc)
 {
 	struct parser_state *state = yyget_extra(scanner);
 	struct error_record *erec;
@@ -696,8 +694,9 @@ static int include_file(void *scanner, const char *filename,
 			     filename, strerror(errno));
 		goto err;
 	}
+	nft->f[state->indesc_idx] = f;
 
-	erec = scanner_push_file(scanner, filename, f, loc);
+	erec = scanner_push_file(nft, scanner, filename, loc);
 	if (erec != NULL)
 		goto err;
 	return 0;
@@ -706,7 +705,7 @@ err:
 	return -1;
 }
 
-static int include_glob(void *scanner, const char *pattern,
+static int include_glob(struct nft_ctx *nft, void *scanner, const char *pattern,
 			const struct location *loc)
 {
 	struct parser_state *state = yyget_extra(scanner);
@@ -770,7 +769,7 @@ static int include_glob(void *scanner, const char *pattern,
 			if (len == 0 || path[len - 1] == '/')
 				continue;
 
-			ret = include_file(scanner, path, loc);
+			ret = include_file(nft, scanner, path, loc);
 			if (ret != 0)
 				goto err;
 		}
@@ -804,10 +803,10 @@ err:
 	return -1;
 }
 
-int scanner_read_file(void *scanner, const char *filename,
+int scanner_read_file(struct nft_ctx *nft, const char *filename,
 		      const struct location *loc)
 {
-	return include_file(scanner, filename, loc);
+	return include_file(nft, nft->scanner, filename, loc);
 }
 
 static bool search_in_include_path(const char *filename)
@@ -837,7 +836,7 @@ int scanner_include_file(struct nft_ctx *nft, void *scanner,
 				return -1;
 			}
 
-			ret = include_glob(scanner, buf, loc);
+			ret = include_glob(nft, scanner, buf, loc);
 
 			/* error was already handled */
 			if (ret == -1)
@@ -852,7 +851,7 @@ int scanner_include_file(struct nft_ctx *nft, void *scanner,
 		}
 	} else {
 		/* an absolute path (starts with '/') */
-		ret = include_glob(scanner, filename, loc);
+		ret = include_glob(nft, scanner, filename, loc);
 	}
 
 	/* handle the case where no file was found */
@@ -897,9 +896,9 @@ void *scanner_init(struct parser_state *state)
 	return scanner;
 }
 
-void scanner_destroy(void *scanner)
+void scanner_destroy(struct nft_ctx *nft)
 {
-	struct parser_state *state = yyget_extra(scanner);
+	struct parser_state *state = yyget_extra(nft->scanner);
 
 	do {
 		struct input_descriptor *inpdesc =
@@ -908,8 +907,13 @@ void scanner_destroy(void *scanner)
 			xfree(inpdesc->name);
 			inpdesc->name = NULL;
 		}
-		yypop_buffer_state(scanner);
+		yypop_buffer_state(nft->scanner);
+
+		if (nft->f[state->indesc_idx]) {
+			fclose(nft->f[state->indesc_idx]);
+			nft->f[state->indesc_idx] = NULL;
+		}
 	} while (state->indesc_idx--);
 
-	yylex_destroy(scanner);
+	yylex_destroy(nft->scanner);
 }
-- 
2.11.0