Adding basic verification to PPDs selected by the user

When the user selects a file to be used as a PPD for a printer to be
added, a simple check is made to verify if the selected file looks to be
a PPD. This does not perform a full validation of the PPD file and is
just to help notify the user if they've selected a file which is
obviously wrong.

R=skau@chromium.org, xdai@chromium.org

Bug: 778462
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I794a48c84d176b979e9bfdcf90ee5049f08f4541
Reviewed-on: https://chromium-review.googlesource.com/887858
Commit-Queue: David Valleau <valleau@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533792}

chrome/browser/resources/settings/printing_page/cups_set_manufacturer_model_behavior.js

@@ -86,10 +86,12 @@ const SetManufacturerModelBehavior = {
   },
 
   /**
-   * @param {string} path
+   * @param {string} path The full path to the selected PPD file
    * @private
    */
   printerPPDPathChanged_: function(path) {
+    // TODO(valleau): Display an error message to users
+    // (https://crbug.com/806915)
     this.set('activePrinter.printerPPDPath', path);
   },
 

chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.cc

@@ -8,12 +8,14 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/containers/flat_map.h"
 #include "base/files/file_util.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
+#include "base/task_scheduler/post_task.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
@@ -32,6 +34,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/debug_daemon_client.h"
 #include "chromeos/printing/ppd_cache.h"
+#include "chromeos/printing/ppd_line_reader.h"
 #include "chromeos/printing/ppd_provider.h"
 #include "chromeos/printing/printer_configuration.h"
 #include "chromeos/printing/printing_constants.h"
@@ -53,6 +56,8 @@ namespace {
 // enums must never be renumbered or deleted and reused.
 enum PpdSourceForHistogram { kUser = 0, kScs = 1, kPpdSourceMax };
 
+constexpr int kPpdMaxLineLength = 255;
+
 void RecordPpdSource(const PpdSourceForHistogram& source) {
   UMA_HISTOGRAM_ENUMERATION("Printing.CUPS.PpdSource", source, kPpdSourceMax);
 }
@@ -236,6 +241,16 @@ std::unique_ptr<chromeos::Printer> DictToPrinter(
   return printer;
 }
 
+std::string ReadFileToStringWithMaxSize(const base::FilePath& path,
+                                        int max_size) {
+  std::string contents;
+  // This call can fail, but it doesn't matter for our purposes. If it fails,
+  // we simply return an empty string for the contents, and it will be rejected
+  // as an invalid PPD.
+  base::ReadFileToStringWithMaxSize(path, &contents, max_size);
+  return contents;
+}
+
 }  // namespace
 
 CupsPrintersHandler::CupsPrintersHandler(content::WebUI* webui)
@@ -720,8 +735,26 @@ void CupsPrintersHandler::FileSelected(const base::FilePath& path,
                                        int index,
                                        void* params) {
   DCHECK(!webui_callback_id_.empty());
+
+  // Load the beggining contents of the file located at |path| and callback into
+  // VerifyPpdContents() in order to determine whether the file appears to be a
+  // PPD file. The task's priority is USER_BLOCKING because the this task
+  // updates the UI as a result of a direct user action.
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_BLOCKING},
+      base::BindOnce(&ReadFileToStringWithMaxSize, path, kPpdMaxLineLength),
+      base::BindOnce(&CupsPrintersHandler::VerifyPpdContents,
+                     weak_factory_.GetWeakPtr(), path));
+}
+
+void CupsPrintersHandler::VerifyPpdContents(const base::FilePath& path,
+                                            const std::string& contents) {
+  std::string result = "";
+  if (PpdLineReader::ContainsMagicNumber(contents, kPpdMaxLineLength))
+    result = path.value();
+
   ResolveJavascriptCallback(base::Value(webui_callback_id_),
-                            base::Value(path.value()));
+                            base::Value(result));
   webui_callback_id_.clear();
 }
 

chrome/browser/ui/webui/settings/chromeos/cups_printers_handler.h

@@ -145,6 +145,13 @@ class CupsPrintersHandler : public ::settings::SettingsPageUIHandler,
                     int index,
                     void* params) override;
 
+  // Used by FileSelected() in order to verify whether the beginning contents of
+  // the selected file contain the magic number present in all PPD files. |path|
+  // is used for display in the UI as this function calls back into javascript
+  // with |path| as the result.
+  void VerifyPpdContents(const base::FilePath& path,
+                         const std::string& contents);
+
   Profile* profile_;
 
   // Discovery support.  discovery_active_ tracks whether or not the UI

chromeos/printing/ppd_line_reader.cc

@@ -8,6 +8,7 @@
 #include <string>
 #include <utility>
 
+#include "base/strings/string_util.h"
 #include "net/base/io_buffer.h"
 #include "net/filter/gzip_header.h"
 #include "net/filter/gzip_source_stream.h"
@@ -16,6 +17,8 @@
 namespace chromeos {
 namespace {
 
+constexpr char kPPDMagicNumberString[] = "*PPD-Adobe:";
+
 // Return true if contents has a valid Gzip header.
 bool IsGZipped(const std::string& contents) {
   const char* unused;
@@ -174,4 +177,14 @@ std::unique_ptr<PpdLineReader> PpdLineReader::Create(
   return std::make_unique<PpdLineReaderImpl>(contents, max_line_length);
 }
 
+// static
+bool PpdLineReader::ContainsMagicNumber(const std::string& contents,
+                                        int max_line_length) {
+  auto line_reader = PpdLineReader::Create(contents, max_line_length);
+  std::string line;
+  return line_reader->NextLine(&line) &&
+         base::StartsWith(line, kPPDMagicNumberString,
+                          base::CompareCase::SENSITIVE);
+}
+
 }  // namespace chromeos

chromeos/printing/ppd_line_reader.h

@@ -24,6 +24,14 @@ class CHROMEOS_EXPORT PpdLineReader {
   // unchanged while the Created PpdReader exists.
   static std::unique_ptr<PpdLineReader> Create(const std::string& contents,
                                                int max_line_length);
+
+  // Checks to see whether the file contents in |contents| contains the magic
+  // number which is found at the beginning of every PPD file. To verify this,
+  // a line reader is created which simply attempts to read the first line and
+  // checks whether it contains the magic number.
+  static bool ContainsMagicNumber(const std::string& contents,
+                                  int max_line_length);
+
   virtual ~PpdLineReader() = default;
 
   // Get the contents of the next non-empty line from the ppd into |line|.

chromeos/printing/ppd_line_reader_unittest.cc

@@ -103,6 +103,8 @@ const char kTestPpdGzipped[] = {
     0x81, 0x77, 0x98, 0xf6, 0xaf, 0x74, 0x2c, 0x7e, 0xda, 0x30, 0xfe, 0x60,
     0xad, 0x3f, 0xe6, 0x78, 0x79, 0x4f, 0xd4, 0x03, 0x00, 0x00};
 
+constexpr int kPpdMaxLineLength = 255;
+
 // Read |ppd_contents| line by line with a reader using |max_line_length| as the
 // line limit.  Expect the read lines to be the same as |expected_lines|.
 void RunTest(const std::string& ppd_contents,
@@ -119,21 +121,22 @@ void RunTest(const std::string& ppd_contents,
 
 // Trivial empty ppd test.
 TEST(PpdLineReaderTest, Empty) {
-  RunTest("", 255, {});
+  RunTest("", kPpdMaxLineLength, {});
 }
 
 // Read kTestPpd contents.
 TEST(PpdLineReaderTest, SimplePpd) {
   std::vector<std::string> expected = base::SplitString(
       kTestPpd, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  RunTest(kTestPpd, 255, expected);
+  RunTest(kTestPpd, kPpdMaxLineLength, expected);
 }
 
 // Same as SimplePpd test, but with gzipped contents.
 TEST(PpdLineReaderTest, SimplePpdGzipped) {
   std::vector<std::string> expected = base::SplitString(
       kTestPpd, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
-  RunTest(std::string(kTestPpdGzipped, sizeof(kTestPpdGzipped)), 255, expected);
+  RunTest(std::string(kTestPpdGzipped, sizeof(kTestPpdGzipped)),
+          kPpdMaxLineLength, expected);
 }
 
 // Test that we skip lines exceeding max_line_length.
@@ -169,7 +172,7 @@ TEST(PpdLineReaderTest, CorruptGzipData) {
     gzipped_contents[i] = 0;
   }
 
-  auto reader = PpdLineReader::Create(gzipped_contents, 255);
+  auto reader = PpdLineReader::Create(gzipped_contents, kPpdMaxLineLength);
   // Read as far as we can before the gzip stream gives up.
   std::string unused;
 
@@ -181,5 +184,41 @@ TEST(PpdLineReaderTest, CorruptGzipData) {
   EXPECT_TRUE(reader->Error());
 }
 
+// Tests that the simple PPD begins with the magic number which is present at
+// the beginning of all PPD files.
+TEST(PpdLineReaderTest, SimplePpdContainsMagicNumber) {
+  EXPECT_TRUE(PpdLineReader::ContainsMagicNumber(kTestPpd, kPpdMaxLineLength));
+}
+
+// Tests that the Gzipped version of the PPD file begins with the magic number.
+TEST(PpdLineReaderTest, GzippedPpdContainsMagicNumber) {
+  const std::string gzipped_contents(kTestPpdGzipped, sizeof(kTestPpdGzipped));
+  EXPECT_TRUE(
+      PpdLineReader::ContainsMagicNumber(gzipped_contents, kPpdMaxLineLength));
+}
+
+// Tests that a file which does not begin with the PPD magic number will be
+// rejected.
+TEST(PpdLineReaderTest, InvalidPpdMagicNumber) {
+  EXPECT_FALSE(PpdLineReader::ContainsMagicNumber(
+      "*%%%% PPD file for HP PSC 900 Series with CUPS.", kPpdMaxLineLength));
+
+  EXPECT_FALSE(PpdLineReader::ContainsMagicNumber("MZ this is fake exe file",
+                                                  kPpdMaxLineLength));
+  EXPECT_FALSE(PpdLineReader::ContainsMagicNumber("PK this is a fake zip file",
+                                                  kPpdMaxLineLength));
+}
+
+// Tests that a file which begins with newline characters should still be
+// rejected even if it contains the magic number since it does not appear at the
+// beginning of the file.
+TEST(PpdLineReaderTest, RejectFileStartingWithNewline) {
+  EXPECT_FALSE(PpdLineReader::ContainsMagicNumber("\x0A*PPD-Adobe: \"4.3\"",
+                                                  kPpdMaxLineLength));
+
+  EXPECT_FALSE(PpdLineReader::ContainsMagicNumber("\x0D*PPD-Adobe: \"4.3\"",
+                                                  kPpdMaxLineLength));
+}
+
 }  // namespace
 }  // namespace chromeos