Merge pull request #661 from mailpoet/security_issue_633

Import SQL injection

lib/Subscribers/ImportExport/Import/Import.php

@@ -21,6 +21,7 @@ class Import {
   public $updated_at;
 
   public function __construct($data) {
+    $this->validateData($data);
     $this->subscribers_data = $this->transformSubscribersData(
       $data['subscribers'],
       $data['columns']
@@ -41,6 +42,23 @@ class Import {
     $this->updated_at = date('Y-m-d H:i:s', (int)$data['timestamp'] + 1);
   }
 
+  function validateData($data) {
+    $required_data_fields = array(
+      'subscribers',
+      'columns',
+      'segments',
+      'timestamp',
+      'updateSubscribers'
+    );
+    // 1. data should contain all required fields
+    // 2. column names should only contain alphanumeric & underscore characters
+    if(count(array_intersect_key(array_flip($required_data_fields), $data)) !== count($required_data_fields) ||
+       preg_grep('/[^a-zA-Z0-9_]/', array_keys($data['columns']))
+    ) {
+      throw new \Exception(__('Missing or invalid subscriber data.', 'mailpoet'));
+    }
+  }
+
   function getSubscriberFieldsValidationRules($subscriber_fields) {
     $validation_rules = array();
     foreach($subscriber_fields as $column => $field) {
@@ -89,8 +107,8 @@ class Import {
           $this->synchronizeWPUsers($wp_users);
         }
       }
-    } catch(\PDOException $e) {
-      throw new \Exception($e->getMessage());
+    } catch(\Exception $e) {
+      throw new \Exception(__('Unable to save imported subscribers.', 'mailpoet'));
     }
     $import_factory = new ImportExportFactory('import');
     $segments = $import_factory->getSegments();
@@ -364,6 +382,11 @@ class Import {
     $subscribers_data,
     $subscriber_custom_fields
   ) {
+    // check if custom fields exist in the database
+    $subscriber_custom_fields = Helpers::flattenArray(
+      CustomField::whereIn('id', $subscriber_custom_fields)->select('id')->findArray()
+    );
+    if(!$subscriber_custom_fields) return;
     $subscribers = array_map(
       function($column) use ($db_subscribers, $subscribers_data) {
         $count = range(0, count($subscribers_data[$column]) - 1);
@@ -406,4 +429,4 @@ class Import {
       );
     }
   }
-}
+}

tests/unit/Subscribers/ImportExport/Import/ImportTest.php

@@ -66,6 +66,31 @@ class ImportTest extends MailPoetTest {
     expect($this->import->updated_at)->notEmpty();
   }
 
+  function testItChecksForRequiredDataFields() {
+    $data = $this->data;
+    // exception should be thrown when one or more fields do not exist
+    unset($data['timestamp']);
+    try {
+      $this->import->validateData($data);
+      self::fail('Missing or invalid data exception not thrown.');
+    } catch(Exception $e) {
+      expect($e->getMessage())->equals('Missing or invalid subscriber data.');
+    }
+    // exception should not be thrown when all fields exist
+    $this->import->validateData($this->data);
+  }
+
+  function testItValidatesColumnNames() {
+    $data = $this->data;
+    $data['columns']['test) values ((ExtractValue(1,CONCAT(0x5c, (SELECT version())))))%23'] = true;
+    try {
+      $this->import->validateData($data);
+      self::fail('Missing or invalid data exception not thrown.');
+    } catch(Exception $e) {
+      expect($e->getMessage())->equals('Missing or invalid subscriber data.');
+    }
+  }
+
   function testItCanTransformSubscribers() {
     $custom_field = $this->subscriber_custom_fields[0];
     expect($this->import->subscribers_data['first_name'][0])