Move shebang handling into the lexer

Instead of handling shebang lines by adjusting the file pointer in individual SAPIs, move the handling into the lexer, where this is both a lot simpler and more robust. Whether the shebang should be skipped is controlled by CG(skip_shebang) -- we might want to do that in more cases. This fixed bugs #60677 and #78066.
php · Jul 15, 2019 · c5f1b38 · c5f1b38
1 parent 17d4e86
commit c5f1b38
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 170 deletions.
diff --git a/NEWS b/NEWS
@@ -4,6 +4,10 @@ PHP                                                                        NEWS
 
 - Core:
   . Fixed bug #78212 (Segfault in built-in webserver). (cmb)
+  . Fixed bug #60677 (CGI doesn't properly validate shebang line contains #!).
+    (Nikita)
+  . Fixed bug #78066 (PHP eats the first byte of a program that comes from
+    process substitution). (Nikita)
 
 - Libxml:
   . Fixed bug #78279 (libxml_disable_entity_loader settings is shared between

diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c
@@ -329,7 +329,7 @@ void zend_init_compiler_data_structures(void) /* {{{ */
 	zend_stack_init(&CG(delayed_oplines_stack), sizeof(zend_op));
 	CG(active_class_entry) = NULL;
 	CG(in_compilation) = 0;
-	CG(start_lineno) = 0;
+	CG(skip_shebang) = 0;
 
 	CG(encoding_declared) = 0;
 	CG(memoized_exprs) = NULL;

diff --git a/Zend/zend_globals.h b/Zend/zend_globals.h
@@ -93,7 +93,7 @@ struct _zend_compiler_globals {
 
 	struct _zend_ini_parser_param *ini_parser_param;
 
-	uint32_t start_lineno;
+	zend_bool skip_shebang;
 	zend_bool increment_lineno;
 
 	zend_string *doc_comment;

diff --git a/Zend/zend_language_scanner.l b/Zend/zend_language_scanner.l
@@ -513,16 +513,9 @@ ZEND_API int zend_multibyte_set_filter(const zend_encoding *onetime_encoding)
 ZEND_API int open_file_for_scanning(zend_file_handle *file_handle)
 {
 	char *buf;
-	size_t size, offset = 0;
+	size_t size;
 	zend_string *compiled_filename;
 
-	/* The shebang line was read, get the current position to obtain the buffer start */
-	if (CG(start_lineno) == 2 && file_handle->type == ZEND_HANDLE_FP && file_handle->handle.fp) {
-		if ((offset = ftell(file_handle->handle.fp)) == (size_t)-1) {
-			offset = 0;
-		}
-	}
-
 	if (zend_stream_fixup(file_handle, &buf, &size) == FAILURE) {
 		return FAILURE;
 	}
@@ -556,13 +549,18 @@ ZEND_API int open_file_for_scanning(zend_file_handle *file_handle)
 				size = SCNG(script_filtered_size);
 			}
 		}
-		SCNG(yy_start) = (unsigned char *)buf - offset;
+		SCNG(yy_start) = (unsigned char *)buf;
 		yy_scan_buffer(buf, (unsigned int)size);
 	} else {
 		zend_error_noreturn(E_COMPILE_ERROR, "zend_stream_mmap() failed");
 	}
 
-	BEGIN(INITIAL);
+	if (CG(skip_shebang)) {
+		CG(skip_shebang) = 0;
+		BEGIN(SHEBANG);
+	} else {
+		BEGIN(INITIAL);
+	}
 
 	if (file_handle->opened_path) {
 		compiled_filename = zend_string_copy(file_handle->opened_path);
@@ -573,14 +571,8 @@ ZEND_API int open_file_for_scanning(zend_file_handle *file_handle)
 	zend_set_compiled_filename(compiled_filename);
 	zend_string_release_ex(compiled_filename, 0);
 
-	if (CG(start_lineno)) {
-		CG(zend_lineno) = CG(start_lineno);
-		CG(start_lineno) = 0;
-	} else {
-		CG(zend_lineno) = 1;
-	}
-
 	RESET_DOC_COMMENT();
+	CG(zend_lineno) = 1;
 	CG(increment_lineno) = 0;
 	return SUCCESS;
 }
@@ -2009,6 +2001,17 @@ string:
 	RETURN_TOKEN(T_NS_C);
 }
 
+<SHEBANG>"#!" .* {NEWLINE} {
+	CG(zend_lineno)++;
+	BEGIN(INITIAL);
+	goto restart;
+}
+
+<SHEBANG>{ANY_CHAR} {
+	yyless(0);
+	BEGIN(INITIAL);
+	goto restart;
+}
 
 <INITIAL>"<?=" {
 	BEGIN(ST_IN_SCRIPTING);

diff --git a/main/main.c b/main/main.c
@@ -2644,15 +2644,13 @@ PHPAPI int php_execute_script(zend_file_handle *primary_file)
 
 		/*
 		   If cli primary file has shabang line and there is a prepend file,
-		   the `start_lineno` will be used by prepend file but not primary file,
+		   the `skip_shebang` will be used by prepend file but not primary file,
 		   save it and restore after prepend file been executed.
 		 */
-		if (CG(start_lineno) && prepend_file_p) {
-			int orig_start_lineno = CG(start_lineno);
-
-			CG(start_lineno) = 0;
+		if (CG(skip_shebang) && prepend_file_p) {
+			CG(skip_shebang) = 0;
 			if (zend_execute_scripts(ZEND_REQUIRE, NULL, 1, prepend_file_p) == SUCCESS) {
-				CG(start_lineno) = orig_start_lineno;
+				CG(skip_shebang) = 1;
 				retval = (zend_execute_scripts(ZEND_REQUIRE, NULL, 2, primary_file, append_file_p) == SUCCESS);
 			}
 		} else {

diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c
@@ -2550,78 +2550,7 @@ consult the installation file that came with this distribution, or visit \n\
 			}
 
 			if (CGIG(check_shebang_line)) {
-				/* #!php support */
-				switch (file_handle.type) {
-					case ZEND_HANDLE_FD:
-						if (file_handle.handle.fd < 0) {
-							break;
-						}
-						file_handle.type = ZEND_HANDLE_FP;
-						file_handle.handle.fp = fdopen(file_handle.handle.fd, "rb");
-						/* break missing intentionally */
-					case ZEND_HANDLE_FP:
-						if (!file_handle.handle.fp ||
-						    (file_handle.handle.fp == stdin)) {
-							break;
-						}
-						c = fgetc(file_handle.handle.fp);
-						if (c == '#') {
-							while (c != '\n' && c != '\r' && c != EOF) {
-								c = fgetc(file_handle.handle.fp);	/* skip to end of line */
-							}
-							/* handle situations where line is terminated by \r\n */
-							if (c == '\r') {
-								if (fgetc(file_handle.handle.fp) != '\n') {
-									zend_long pos = zend_ftell(file_handle.handle.fp);
-									zend_fseek(file_handle.handle.fp, pos - 1, SEEK_SET);
-								}
-							}
-							CG(start_lineno) = 2;
-						} else {
-							rewind(file_handle.handle.fp);
-						}
-						break;
-					case ZEND_HANDLE_STREAM:
-						c = php_stream_getc((php_stream*)file_handle.handle.stream.handle);
-						if (c == '#') {
-							while (c != '\n' && c != '\r' && c != EOF) {
-								c = php_stream_getc((php_stream*)file_handle.handle.stream.handle);	/* skip to end of line */
-							}
-							/* handle situations where line is terminated by \r\n */
-							if (c == '\r') {
-								if (php_stream_getc((php_stream*)file_handle.handle.stream.handle) != '\n') {
-									zend_off_t pos = php_stream_tell((php_stream*)file_handle.handle.stream.handle);
-									php_stream_seek((php_stream*)file_handle.handle.stream.handle, pos - 1, SEEK_SET);
-								}
-							}
-							CG(start_lineno) = 2;
-						} else {
-							php_stream_rewind((php_stream*)file_handle.handle.stream.handle);
-						}
-						break;
-					case ZEND_HANDLE_MAPPED:
-						if (file_handle.handle.stream.mmap.buf[0] == '#') {
-						    size_t i = 1;
-
-						    c = file_handle.handle.stream.mmap.buf[i++];
-							while (c != '\n' && c != '\r' && i < file_handle.handle.stream.mmap.len) {
-								c = file_handle.handle.stream.mmap.buf[i++];
-							}
-							if (c == '\r') {
-								if (i < file_handle.handle.stream.mmap.len && file_handle.handle.stream.mmap.buf[i] == '\n') {
-									i++;
-								}
-							}
-							if(i > file_handle.handle.stream.mmap.len) {
-								i = file_handle.handle.stream.mmap.len;
-							}
-							file_handle.handle.stream.mmap.buf += i;
-							file_handle.handle.stream.mmap.len -= i;
-						}
-						break;
-					default:
-						break;
-				}
+				CG(skip_shebang) = 1;
 			}
 
 			switch (behavior) {

diff --git a/sapi/cgi/tests/bug60677.phpt b/sapi/cgi/tests/bug60677.phpt
@@ -0,0 +1,9 @@
+--TEST--
+Bug #60677: CGI doesn't properly validate shebang line contains #!
+--CGI--
+--FILE--
+#<?php echo "Hello World\n"; ?>
+Second line.
+--EXPECT--
+#Hello World
+Second line.
diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c
@@ -587,12 +587,9 @@ static const char *param_mode_conflict = "Either execute direct code, process st
 
 /* {{{ cli_seek_file_begin
  */
-static int cli_seek_file_begin(zend_file_handle *file_handle, char *script_file, int *lineno)
+static int cli_seek_file_begin(zend_file_handle *file_handle, char *script_file)
 {
-	int c;
-
-	*lineno = 1;
-
+	// TODO: Is this still needed?
 	file_handle->type = ZEND_HANDLE_FP;
 	file_handle->opened_path = NULL;
 	file_handle->free_filename = 0;
@@ -602,23 +599,7 @@ static int cli_seek_file_begin(zend_file_handle *file_handle, char *script_file,
 	}
 	file_handle->filename = script_file;
 
-	/* #!php support */
-	c = fgetc(file_handle->handle.fp);
-	if (c == '#' && (c = fgetc(file_handle->handle.fp)) == '!') {
-		while (c != '\n' && c != '\r' && c != EOF) {
-			c = fgetc(file_handle->handle.fp);	/* skip to end of line */
-		}
-		/* handle situations where line is terminated by \r\n */
-		if (c == '\r') {
-			if (fgetc(file_handle->handle.fp) != '\n') {
-				zend_long pos = zend_ftell(file_handle->handle.fp);
-				zend_fseek(file_handle->handle.fp, pos - 1, SEEK_SET);
-			}
-		}
-		*lineno = 2;
-	} else {
-		rewind(file_handle->handle.fp);
-	}
+	rewind(file_handle->handle.fp);
 
 	return SUCCESS;
 }
@@ -649,7 +630,6 @@ static int do_cli(int argc, char **argv) /* {{{ */
 	char *arg_free=NULL, **arg_excp=&arg_free;
 	char *script_file=NULL, *translated_path = NULL;
 	int interactive=0;
-	int lineno = 0;
 	const char *param_error=NULL;
 	int hide_argv = 0;
 
@@ -922,7 +902,7 @@ static int do_cli(int argc, char **argv) /* {{{ */
 			php_optind++;
 		}
 		if (script_file) {
-			if (cli_seek_file_begin(&file_handle, script_file, &lineno) != SUCCESS) {
+			if (cli_seek_file_begin(&file_handle, script_file) != SUCCESS) {
 				goto err;
 			} else {
 				char real_path[MAXPATHLEN];
@@ -960,7 +940,7 @@ static int do_cli(int argc, char **argv) /* {{{ */
 			goto err;
 		}
 		request_started = 1;
-		CG(start_lineno) = lineno;
+		CG(skip_shebang) = 1;
 
 		zend_register_bool_constant(
 			ZEND_STRL("PHP_CLI_PROCESS_TITLE"),
@@ -1050,10 +1030,10 @@ static int do_cli(int argc, char **argv) /* {{{ */
 						}
 					} else {
 						if (script_file) {
-							if (cli_seek_file_begin(&file_handle, script_file, &lineno) != SUCCESS) {
+							if (cli_seek_file_begin(&file_handle, script_file) != SUCCESS) {
 								exit_status = 1;
 							} else {
-								CG(start_lineno) = lineno;
+								CG(skip_shebang) = 1;
 								php_execute_script(&file_handle);
 								exit_status = EG(exit_status);
 							}

diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c
@@ -567,62 +567,17 @@ int phpdbg_compile(void) /* {{{ */
 {
 	zend_file_handle fh;
 	char *buf;
-	char *start_line = NULL;
 	size_t len;
-	size_t start_line_len;
-	int i;
 
 	if (!PHPDBG_G(exec)) {
 		phpdbg_error("inactive", "type=\"nocontext\"", "No execution context");
 		return FAILURE;
 	}
 
 	if (php_stream_open_for_zend_ex(PHPDBG_G(exec), &fh, USE_PATH|STREAM_OPEN_FOR_INCLUDE) == SUCCESS && zend_stream_fixup(&fh, &buf, &len) == SUCCESS) {
-		/* Skip #! line */
-		if (len >= 3 && buf[0] == '#' && buf[1] == '!') {
-			char *end = buf + len;
-			do {
-				switch (fh.handle.stream.mmap.buf++[0]) {
-					case '\r':
-						if (fh.handle.stream.mmap.buf[0] == '\n') {
-							fh.handle.stream.mmap.buf++;
-						}
-					case '\n':
-						CG(start_lineno) = 2;
-						start_line_len = fh.handle.stream.mmap.buf - buf;
-						start_line = emalloc(start_line_len);
-						memcpy(start_line, buf, start_line_len);
-						fh.handle.stream.mmap.len -= start_line_len;
-						end = fh.handle.stream.mmap.buf;
-				}
-			} while (fh.handle.stream.mmap.buf + 1 < end);
-		}
-
+		CG(skip_shebang) = 1;
 		PHPDBG_G(ops) = zend_compile_file(&fh, ZEND_INCLUDE);
 
-		/* prepend shebang line to file_source */
-		if (start_line) {
-			phpdbg_file_source *data = zend_hash_find_ptr(&PHPDBG_G(file_sources), PHPDBG_G(ops)->filename);
-
-			dtor_func_t dtor = PHPDBG_G(file_sources).pDestructor;
-			PHPDBG_G(file_sources).pDestructor = NULL;
-			zend_hash_del(&PHPDBG_G(file_sources), PHPDBG_G(ops)->filename);
-			PHPDBG_G(file_sources).pDestructor = dtor;
-
-			data = erealloc(data, sizeof(phpdbg_file_source) + sizeof(uint32_t) * ++data->lines);
-			memmove(data->line + 1, data->line, sizeof(uint32_t) * data->lines);
-			data->line[0] = 0;
-			data->buf = erealloc(data->buf, data->len + start_line_len);
-			memmove(data->buf + start_line_len, data->buf, data->len);
-			memcpy(data->buf, start_line, start_line_len);
-			efree(start_line);
-			data->len += start_line_len;
-			for (i = 1; i <= data->lines; i++) {
-				data->line[i] += start_line_len;
-			}
-			zend_hash_update_ptr(&PHPDBG_G(file_sources), PHPDBG_G(ops)->filename, data);
-		}
-
 		fh.handle.stream.mmap.buf = buf;
 		fh.handle.stream.mmap.len = len;
 		zend_destroy_file_handle(&fh);