From 138a5e968ea6ba67e1cf9f2c750a0e35ea89345a Mon Sep 17 00:00:00 2001 From: Fotis Papadopoulos Date: Mon, 9 Mar 2015 14:52:56 +0200 Subject: [PATCH 1/3] Improved compiler request validation function, making its response more verbose, in case of an error --- .../Handler/PreprocessingHandler.php | 113 +++++++++++++----- 1 file changed, 81 insertions(+), 32 deletions(-) diff --git a/Symfony/src/Codebender/CompilerBundle/Handler/PreprocessingHandler.php b/Symfony/src/Codebender/CompilerBundle/Handler/PreprocessingHandler.php index 720c2dd..3d7e5d4 100644 --- a/Symfony/src/Codebender/CompilerBundle/Handler/PreprocessingHandler.php +++ b/Symfony/src/Codebender/CompilerBundle/Handler/PreprocessingHandler.php @@ -219,7 +219,7 @@ function build_code($code, $function_prototypes, $position) return $return_code; } - function ino_to_cpp($code, $filename = NULL) + function ino_to_cpp($code, $filename = null) { // Remove comments, preprocessor directives, single- and double- quotes $no_comms_code = $this->remove_comments_directives_quotes($code); @@ -245,52 +245,101 @@ function ino_to_cpp($code, $filename = NULL) } - - - /** - \brief Decodes and performs validation checks on input data. - - \param string $request The JSON-encoded compile request. - \return The value encoded in JSON in appropriate PHP type or NULL. - */ - function validate_input($request) + /** + * Decodes and performs validation checks on input data. + * + * @param string $request The JSON-encoded compile request. + * @return array The value encoded in JSON in appropriate PHP type or an array containing the error that occurred. + */ + function validate_input(&$request) { $request = json_decode($request, true); // Request must be successfully decoded. - if ($request === NULL) - return NULL; - // Request must contain certain entities. - if (!(array_key_exists("format", $request) - && array_key_exists("version", $request) - && array_key_exists("build", $request) - && array_key_exists("files", $request) - && is_array($request["build"]) - && array_key_exists("libraries", $request) - && array_key_exists("mcu", $request["build"]) - && array_key_exists("f_cpu", $request["build"]) - && array_key_exists("core", $request["build"]) - && is_array($request["files"])) - ) - return NULL; + if ($request === null || $request === false) { + $request = array(); + return array('success' => false, 'error' => 'Request JSON decode: ' . json_last_error()); + } + + $isValidRequest = $this->containsNecessaryEntities($request); + if ($isValidRequest['success'] === false) { + return $isValidRequest; + } // Leonardo-specific flags. if (array_key_exists("variant", $request["build"]) && $request["build"]["variant"] == "leonardo") if (!(array_key_exists("vid", $request["build"]) && array_key_exists("pid", $request["build"])) - ) - return NULL; + ) { + return array('success' => false, 'error' => 'Invalid atmega32u4 board build'); + } // Values used as command-line arguments may not contain any special // characters. This is a serious security risk. $values = array("version", "mcu", "f_cpu", "core", "vid", "pid"); - if (array_key_exists("variant", $request["build"])) + if (array_key_exists("variant", $request["build"])) { $values[] = "variant"; - foreach ($values as $i) - if (isset($request["build"][$i]) && escapeshellcmd($request["build"][$i]) != $request["build"][$i]) - return NULL; + } + + foreach ($values as $i) { + if (isset($request["build"][$i]) && escapeshellcmd($request["build"][$i]) != $request["build"][$i]) { + return array('success' => false, 'error' => 'Security issue raised because ' . $i . ' build entity contains special chars.'); + } + } // Request is valid. - return $request; + return array('success' => true); } + + /** + * Makes sure the request contains all the necessary entities needed in order to + * proceed with the compilation. + * + * @param array $request + * @return array + */ + function containsNecessaryEntities($request) + { + $entities = array('format', 'version', 'build', 'files', 'libraries'); + $buildEntities = array('mcu', 'f_cpu', 'core'); + $missingEntities = array(); + $missingBuildEntities = array(); + + foreach ($entities as $entity) { + if (!array_key_exists($entity, $request)) { + $missingEntities[] = $entity; + } + } + + if(count($missingEntities) > 0) { + return array('success' => false, 'error' => 'Missing entities from request: ' . implode(', ', $missingEntities)); + } + + if (!is_array($request['files'])) { + return array('success' => false, 'error' => 'Invalid files format in request.'); + } + + // Each of the files must be in a [filename --> content] format + foreach ($request['files'] as $file) { + if (!array_key_exists('filename', $file) || !array_key_exists('content', $file)) { + return array('success' => false, 'error' => 'One of the files is not in [filename -> content] format'); + } + } + + if (!is_array($request['build'])) { + return array('success' => false, 'error' => 'Invalid build format in request.'); + } + + foreach ($buildEntities as $entity) { + if (!array_key_exists($entity, $request['build'])) { + $missingBuildEntities[] = $entity; + } + } + + if (count($missingBuildEntities) > 0) { + return array('success' => false, 'error' => 'Missing entities from board build: ' . implode(', ', $missingBuildEntities)); + } + + return array('success' => true); + } } \ No newline at end of file From d314d3d83afe7d6a19c2ea4b20a42a95b60f65c2 Mon Sep 17 00:00:00 2001 From: Fotis Papadopoulos Date: Mon, 9 Mar 2015 15:14:03 +0200 Subject: [PATCH 2/3] Excluded .txt files from files extraction --- .../Codebender/CompilerBundle/Handler/UtilityHandler.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Symfony/src/Codebender/CompilerBundle/Handler/UtilityHandler.php b/Symfony/src/Codebender/CompilerBundle/Handler/UtilityHandler.php index b539f31..497084a 100644 --- a/Symfony/src/Codebender/CompilerBundle/Handler/UtilityHandler.php +++ b/Symfony/src/Codebender/CompilerBundle/Handler/UtilityHandler.php @@ -57,6 +57,13 @@ function extract_files($directory, $request_files, $lib_extraction) $content = $file["content"]; $ignore = false; + /* + * We use .txt files in order to recognize the user and project ids, + * so we don't need to create error logs for these files. + */ + if (pathinfo($filename, PATHINFO_EXTENSION) == 'txt') + continue; + $failure_response = array( "success" => false, "step" => 1, From 02d8f926a210a8624203c3b23e3203cd177c7552 Mon Sep 17 00:00:00 2001 From: Fotis Papadopoulos Date: Mon, 9 Mar 2015 15:15:46 +0200 Subject: [PATCH 3/3] Updated the CompilerHandler code in order to enable logging errors that occur in input validation and file extraction --- .../Handler/CompilerHandler.php | 67 ++++++++++++++++--- 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/Symfony/src/Codebender/CompilerBundle/Handler/CompilerHandler.php b/Symfony/src/Codebender/CompilerBundle/Handler/CompilerHandler.php index 653c5cd..536d9fc 100644 --- a/Symfony/src/Codebender/CompilerBundle/Handler/CompilerHandler.php +++ b/Symfony/src/Codebender/CompilerBundle/Handler/CompilerHandler.php @@ -48,18 +48,30 @@ function main($request, $compiler_config) { error_reporting(E_ALL & ~E_STRICT); + // Keep track of the time it takes to compile the project + $start_time = microtime(true); + + /* + * Initialize logger id + * In case an error occurs before we get the user and project ids, + * the log id depends on the current timestamp and some fixed string. + */ + $this->logger_id = $start_time . '_' . 'DEFAULT_LOG_ID'; + $this->set_values($compiler_config, $BINUTILS, $CLANG, $CFLAGS, $CPPFLAGS, $ASFLAGS, $ARFLAGS, $LDFLAGS, $LDFLAGS_TAIL, $CLANG_FLAGS, $OBJCOPY_FLAGS, $SIZE_FLAGS, $OUTPUT, $ARDUINO_CORES_DIR, $EXTERNAL_CORES_DIR, $TEMP_DIR, $ARCHIVE_DIR, $AUTOCC_DIR, $PYTHON, $AUTOCOMPLETER); - $start_time = microtime(true); - // Step 0: Reject the request if the input data is not valid. //TODO: Replace $tmp variable name $tmp = $this->requestValid($request); - if($tmp["success"] === false) + $this->setLoggerId($start_time, $request); + + if($tmp["success"] === false) { + $this->compiler_logger->addInfo($this->logger_id . " - " . $tmp['error']); return $tmp; + } $this->set_variables($request, $format, $libraries, $version, $mcu, $f_cpu, $core, $variant, $vid, $pid, $compiler_config); @@ -73,8 +85,10 @@ function main($request, $compiler_config) $files = array(); $tmp = $this->extractFiles($request["files"], $TEMP_DIR, $compiler_dir, $files["sketch_files"], "files"); - if ($tmp["success"] == false) + if ($tmp["success"] == false) { + $this->compiler_logger->addInfo($this->logger_id . " - " . 'Project files extraction: ' . $tmp['message']); return $tmp; + } // Add the compiler temp directory to the compiler_config struct. $compiler_config["compiler_dir"] = $compiler_dir; @@ -84,8 +98,10 @@ function main($request, $compiler_config) foreach($libraries as $library => $library_files){ $tmp = $this->extractFiles($library_files, $TEMP_DIR, $compiler_dir, $files["libs"][$library], "libraries/$library", true); - if ($tmp["success"] == false) + if ($tmp["success"] == false) { + $this->compiler_logger->addInfo($this->logger_id . " - " . $library . ' library files extraction: ' . $tmp['message']); return $tmp; + } } if (!array_key_exists("archive", $request) || ($request["archive"] !== false && $request["archive"] !== true)) @@ -137,7 +153,7 @@ function main($request, $compiler_config) } } - $this->logger_id = microtime(true) . "_" . substr($compiler_config['compiler_dir'], -6) . "_user:$user_id" . "_project:$sketch_id"; + $this->logger_id = $start_time . "_" . substr($compiler_config['compiler_dir'], -6) . "_user:$user_id" . "_project:$sketch_id"; $this->compiler_logger->addInfo($this->logger_id . " - " . implode(" ", $req_elements)); if ($ARCHIVE_OPTION === true) @@ -406,15 +422,46 @@ function main($request, $compiler_config) private function requestValid(&$request) { - $request = $this->preproc->validate_input($request); - if (!$request) + $response = $this->preproc->validate_input($request); + if ($response['success'] === false) { return array( "success" => false, "step" => 0, - "message" => "Invalid input."); - else return array("success" => true); + "message" => "Invalid input.", + "error" => $response['error']); + } + + return array("success" => true); } + /** + * Sets the compiler logger id, if project id and user id files exist in the request + * + * @param $timestamp + * @param $request + */ + private function setLoggerId($timestamp, $request) + { + if (!array_key_exists('files', $request)) { + return; + } + + $user_id = 'null'; + $sketch_id = 'null'; + + foreach ($request["files"] as $file) { + if (!array_key_exists('filename', $file)) { + continue; + } + if (strpos($file["filename"], ".txt") !== false) { + if (preg_match('/(?<=user_)[\d]+/', $file['filename'], $match)) $user_id = $match[0]; + if (preg_match('/(?<=project_)[\d]+/', $file['filename'], $match)) $sketch_id = $match[0]; + + } + } + $this->logger_id = $timestamp . "_" . "user:$user_id" . "_project:$sketch_id"; + return; + } private function createArchive($compiler_dir, $TEMP_DIR, $ARCHIVE_DIR, &$ARCHIVE_PATH) { if (!file_exists($ARCHIVE_PATH)){