Bug 856042 - Tighten up sanitization of arguments to the Contacts API. r=bz r=gwagner a=tef+
authorReuben Morais <reuben.morais@gmail.com>
Thu, 02 May 2013 13:14:01 -0700
changeset 119254 ab553a6a249394b48da60440e44447644a8d217a
parent 119253 c161ceec64805f51a953d1567b2609474f5c399f
child 119255 499a94cd7ed99b7f8ec4243c9fe587c781c8ec10
push id752
push userreuben.morais@gmail.com
push dateThu, 02 May 2013 20:16:29 +0000
reviewersbz, gwagner, tef
bugs856042
milestone18.0
Bug 856042 - Tighten up sanitization of arguments to the Contacts API. r=bz r=gwagner a=tef+
dom/contacts/ContactManager.js
dom/contacts/ContactManager.manifest
dom/contacts/fallback/ContactDB.jsm
dom/contacts/tests/test_contacts_basics.html
dom/interfaces/contacts/nsIDOMContactProperties.idl
--- a/dom/contacts/ContactManager.js
+++ b/dom/contacts/ContactManager.js
@@ -22,20 +22,36 @@ XPCOMUtils.defineLazyGetter(Services, "D
 XPCOMUtils.defineLazyServiceGetter(this, "pm",
                                    "@mozilla.org/permissionmanager;1",
                                    "nsIPermissionManager");
 
 XPCOMUtils.defineLazyServiceGetter(this, "cpmm",
                                    "@mozilla.org/childprocessmessagemanager;1",
                                    "nsIMessageSender");
 
+function stringOrBust(aObj) {
+  if (typeof aObj != "string") {
+    if (DEBUG) debug("Field is not a string and was ignored.");
+    return undefined;
+  } else {
+    return aObj;
+  }
+}
+
+function sanitizeStringArray(aArray) {
+  if (!Array.isArray(aArray)) {
+    aArray = [aArray];
+  }
+  return aArray.map(stringOrBust).filter(function(el) { return el != undefined; });
+}
+
 const CONTACTS_SENDMORE_MINIMUM = 5;
 
 const nsIClassInfo            = Ci.nsIClassInfo;
-const CONTACTPROPERTIES_CID   = Components.ID("{f5181640-89e8-11e1-b0c4-0800200c9a66}");
+const CONTACTPROPERTIES_CID   = Components.ID("{86504745-da3e-4a17-b67b-88b40e0e6bc8}");
 const nsIDOMContactProperties = Ci.nsIDOMContactProperties;
 
 // ContactProperties is not directly instantiated. It is used as interface.
 
 function ContactProperties(aProp) { if (DEBUG) debug("ContactProperties Constructor"); }
 
 ContactProperties.prototype = {
 
@@ -51,22 +67,22 @@ ContactProperties.prototype = {
 
 //ContactAddress
 
 const CONTACTADDRESS_CONTRACTID = "@mozilla.org/contactAddress;1";
 const CONTACTADDRESS_CID        = Components.ID("{eba48030-89e8-11e1-b0c4-0800200c9a66}");
 const nsIDOMContactAddress      = Components.interfaces.nsIDOMContactAddress;
 
 function ContactAddress(aType, aStreetAddress, aLocality, aRegion, aPostalCode, aCountryName) {
-  this.type = aType || null;
-  this.streetAddress = aStreetAddress || null;
-  this.locality = aLocality || null;
-  this.region = aRegion || null;
-  this.postalCode = aPostalCode || null;
-  this.countryName = aCountryName || null;
+  this.type = sanitizeStringArray(aType);
+  this.streetAddress = stringOrBust(aStreetAddress);
+  this.locality = stringOrBust(aLocality);
+  this.region = stringOrBust(aRegion);
+  this.postalCode = stringOrBust(aPostalCode);
+  this.countryName = stringOrBust(aCountryName);
 };
 
 ContactAddress.prototype = {
   __exposedProps__: {
                       type: 'rw',
                       streetAddress: 'rw',
                       locality: 'rw',
                       region: 'rw',
@@ -86,18 +102,18 @@ ContactAddress.prototype = {
 
 //ContactField
 
 const CONTACTFIELD_CONTRACTID = "@mozilla.org/contactField;1";
 const CONTACTFIELD_CID        = Components.ID("{e2cb19c0-e4aa-11e1-9b23-0800200c9a66}");
 const nsIDOMContactField      = Components.interfaces.nsIDOMContactField;
 
 function ContactField(aType, aValue) {
-  this.type = aType || null;
-  this.value = aValue || null;
+  this.type = sanitizeStringArray(aType);
+  this.value = stringOrBust(aValue);
 };
 
 ContactField.prototype = {
   __exposedProps__: {
                       type: 'rw',
                       value: 'rw'
                      },
 
@@ -113,19 +129,19 @@ ContactField.prototype = {
 
 //ContactTelField
 
 const CONTACTTELFIELD_CONTRACTID = "@mozilla.org/contactTelField;1";
 const CONTACTTELFIELD_CID        = Components.ID("{ed0ab260-e4aa-11e1-9b23-0800200c9a66}");
 const nsIDOMContactTelField      = Components.interfaces.nsIDOMContactTelField;
 
 function ContactTelField(aType, aValue, aCarrier) {
-  this.type = aType || null;
-  this.value = aValue || null;
-  this.carrier = aCarrier || null;
+  this.type = sanitizeStringArray(aType);
+  this.value = stringOrBust(aValue);
+  this.carrier = stringOrBust(aCarrier);
 };
 
 ContactTelField.prototype = {
   __exposedProps__: {
                       type: 'rw',
                       value: 'rw',
                       carrier: 'rw'
                      },
@@ -212,104 +228,121 @@ Contact.prototype = {
                       note: 'rw',
                       impp: 'rw',
                       anniversary: 'rw',
                       sex: 'rw',
                       genderIdentity: 'rw'
                      },
 
   init: function init(aProp) {
-    // Accept non-array strings for DOMString[] properties and convert them.
-    function _create(aField) {
-      if (Array.isArray(aField)) {
-        for (let i = 0; i < aField.length; i++) {
-          if (typeof aField[i] != "string")
-            aField[i] = String(aField[i]);
-        }
-        return aField;
-      } else if (aField != null) {
-        return [String(aField)];
-      }
-    };
-
     function _checkBlobArray(aBlob) {
       if (Array.isArray(aBlob)) {
         for (let i = 0; i < aBlob.length; i++) {
           if (typeof aBlob != 'object') {
             return null;
           }
           if (!(aBlob[i] instanceof Components.interfaces.nsIDOMBlob)) {
             return null;
           }
         }
         return aBlob;
       }
       return null;
-    };
+    }
+
+    function _isVanillaObj(aObj) {
+      return Object.prototype.toString.call(aObj) == "[object Object]";
+    }
+
+    let _create = sanitizeStringArray;
 
-    this.name =            _create(aProp.name) || null;
-    this.honorificPrefix = _create(aProp.honorificPrefix) || null;
-    this.givenName =       _create(aProp.givenName) || null;
-    this.additionalName =  _create(aProp.additionalName) || null;
-    this.familyName =      _create(aProp.familyName) || null;
-    this.honorificSuffix = _create(aProp.honorificSuffix) || null;
-    this.nickname =        _create(aProp.nickname) || null;
+    this.name =            _create(aProp.name);
+    this.honorificPrefix = _create(aProp.honorificPrefix);
+    this.givenName =       _create(aProp.givenName);
+    this.additionalName =  _create(aProp.additionalName);
+    this.familyName =      _create(aProp.familyName);
+    this.honorificSuffix = _create(aProp.honorificSuffix);
+    this.nickname =        _create(aProp.nickname);
 
     if (aProp.email) {
       aProp.email = Array.isArray(aProp.email) ? aProp.email : [aProp.email];
       this.email = new Array();
-      for (let i = 0; i < aProp.email.length; i++)
-        this.email.push(new ContactField(aProp.email[i].type, aProp.email[i].value));
+      for (let email of aProp.email) {
+        if (_isVanillaObj(email)) {
+          this.email.push(new ContactField(email.type, email.value));
+        } else if (DEBUG) {
+          debug("email field is not a ContactField and was ignored.");
+        }
+      }
     } else {
       this.email = null;
     }
 
-    this.photo =           _checkBlobArray(aProp.photo) || null;
-    this.category =        _create(aProp.category) || null;
+    this.photo =           _checkBlobArray(aProp.photo);
+    this.category =        _create(aProp.category);
 
     if (aProp.adr) {
-      // Make sure adr argument is an array. Instanceof doesn't work.
       aProp.adr = Array.isArray(aProp.adr) ? aProp.adr : [aProp.adr];
 
       this.adr = new Array();
-      for (let i = 0; i < aProp.adr.length; i++)
-        this.adr.push(new ContactAddress(aProp.adr[i].type, aProp.adr[i].streetAddress, aProp.adr[i].locality,
-                                         aProp.adr[i].region, aProp.adr[i].postalCode, aProp.adr[i].countryName));
+      for (let adr of aProp.adr) {
+        if (_isVanillaObj(adr)) {
+          this.adr.push(new ContactAddress(adr.type, adr.streetAddress, adr.locality,
+                                           adr.region, adr.postalCode, adr.countryName));
+        } else if (DEBUG) {
+          debug("adr field is not a ContactAddress and was ignored.");
+        }
+      }
     } else {
       this.adr = null;
     }
 
     if (aProp.tel) {
       aProp.tel = Array.isArray(aProp.tel) ? aProp.tel : [aProp.tel];
       this.tel = new Array();
-      for (let i = 0; i < aProp.tel.length; i++)
-        this.tel.push(new ContactTelField(aProp.tel[i].type, aProp.tel[i].value, aProp.tel[i].carrier));
+      for (let tel of aProp.tel) {
+        if (_isVanillaObj(tel)) {
+          this.tel.push(new ContactTelField(tel.type, tel.value, tel.carrier));
+        } else if (DEBUG) {
+          debug("tel field is not a ContactTelField and was ignored.");
+        }
+      }
     } else {
       this.tel = null;
     }
 
-    this.org =             _create(aProp.org) || null;
-    this.jobTitle =        _create(aProp.jobTitle) || null;
+    this.org =             _create(aProp.org);
+    this.jobTitle =        _create(aProp.jobTitle);
     this.bday =            aProp.bday ? new Date(aProp.bday) : null;
-    this.note =            _create(aProp.note) || null;
+    this.note =            _create(aProp.note);
 
     if (aProp.impp) {
       aProp.impp = Array.isArray(aProp.impp) ? aProp.impp : [aProp.impp];
       this.impp = new Array();
-      for (let i = 0; i < aProp.impp.length; i++)
-        this.impp.push(new ContactField(aProp.impp[i].type, aProp.impp[i].value));
+      for (let impp of aProp.impp) {
+        if (_isVanillaObj(impp)) {
+          this.impp.push(new ContactField(impp.type, impp.value));
+        } else if (DEBUG) {
+          debug("impp field is not a ContactField and was ignored.");
+        }
+      }
     } else {
       this.impp = null;
     }
 
     if (aProp.url) {
       aProp.url = Array.isArray(aProp.url) ? aProp.url : [aProp.url];
       this.url = new Array();
-      for (let i = 0; i < aProp.url.length; i++)
-        this.url.push(new ContactField(aProp.url[i].type, aProp.url[i].value));
+      for (let url of aProp.url) {
+        if (_isVanillaObj(url)) {
+          this.url.push(new ContactField(url.type, url.value));
+        } else if (DEBUG) {
+          debug("url field is not a ContactField and was ignored.");
+        }
+      }
     } else {
       this.url = null;
     }
 
     this.anniversary =     aProp.anniversary ? new Date(aProp.anniversary) : null;
     this.sex =             (aProp.sex != "undefined") ? aProp.sex : null;
     this.genderIdentity =  (aProp.genderIdentity != "undefined") ? aProp.genderIdentity : null;
   },
--- a/dom/contacts/ContactManager.manifest
+++ b/dom/contacts/ContactManager.manifest
@@ -1,10 +1,10 @@
-component {f5181640-89e8-11e1-b0c4-0800200c9a66} ContactManager.js
-contract @mozilla.org/contactProperties;1 {f5181640-89e8-11e1-b0c4-0800200c9a66}
+component {86504745-da3e-4a17-b67b-88b40e0e6bc8} ContactManager.js
+contract @mozilla.org/contactProperties;1 {86504745-da3e-4a17-b67b-88b40e0e6bc8}
 
 component {eba48030-89e8-11e1-b0c4-0800200c9a66} ContactManager.js
 contract @mozilla.org/contactAddress;1 {eba48030-89e8-11e1-b0c4-0800200c9a66}
 
 component {e2cb19c0-e4aa-11e1-9b23-0800200c9a66} ContactManager.js
 contract @mozilla.org/contactField;1 {e2cb19c0-e4aa-11e1-9b23-0800200c9a66}
 
 component {ed0ab260-e4aa-11e1-9b23-0800200c9a66} ContactManager.js
--- a/dom/contacts/fallback/ContactDB.jsm
+++ b/dom/contacts/fallback/ContactDB.jsm
@@ -786,24 +786,24 @@ ContactDB.prototype = {
         let result = 0;
         let sortOrder = aFindOptions.sortOrder;
         let sortBy = aFindOptions.sortBy == "familyName" ? [ "familyName", "givenName" ] : [ "givenName" , "familyName" ];
         let xIndex = 0;
         let yIndex = 0;
 
         do {
           while (xIndex < sortBy.length && !x) {
-            x = a.properties[sortBy[xIndex]] ? a.properties[sortBy[xIndex]][0].toLowerCase() : null;
+            x = a.properties[sortBy[xIndex]] && a.properties[sortBy[xIndex]][0] ? a.properties[sortBy[xIndex]][0].toLowerCase() : null;
             xIndex++;
           }
           if (!x) {
-            return sortOrder == 'ascending' ? 1 : -1;
+            return sortOrder == 'descending' ? 1 : -1;
           }
           while (yIndex < sortBy.length && !y) {
-            y = b.properties[sortBy[yIndex]] ? b.properties[sortBy[yIndex]][0].toLowerCase() : null;
+            y = b.properties[sortBy[yIndex]] && b.properties[sortBy[yIndex]][0] ? b.properties[sortBy[yIndex]][0].toLowerCase() : null;
             yIndex++;
           }
           if (!y) {
             return sortOrder == 'ascending' ? 1 : -1;
           }
 
           result = x.localeCompare(y);
           x = null;
--- a/dom/contacts/tests/test_contacts_basics.html
+++ b/dom/contacts/tests/test_contacts_basics.html
@@ -287,35 +287,35 @@ var steps = [
   function () {
     ok(true, "Adding a new contact1");
     createResult1 = new mozContact();
     createResult1.init(properties1);
 
     mozContacts.oncontactchange = function(event) {
       is(event.contactID, createResult1.id, "Same contactID");
       is(event.reason, "create", "Same reason");
+      next();
     }
 
     req = navigator.mozContacts.save(createResult1);
     req.onsuccess = function () {
       ok(createResult1.id, "The contact now has an ID.");
       sample_id1 = createResult1.id;
       checkContacts(properties1, createResult1);
-      next();
     };
     req.onerror = onFailure;
   },
   function () {
     ok(true, "Retrieving by substring");
     var options = {filterBy: ["givenName"],
                    filterOp: "contains",
                    filterValue: properties1.givenName[1].substring(0,3)};
     req = mozContacts.find(options);
     req.onsuccess = function () {
-      ok(req.result.length == 1, "Found exactly 1 contact.");
+      is(req.result.length, 1, "Found exactly 1 contact.");
       findResult1 = req.result[0];
       ok(findResult1.id == sample_id1, "Same ID");
       checkContacts(createResult1, properties1);
       dump("findResult: " + JSON.stringify(findResult1) + "\n");
       // Some manual testing. Testint the testfunctions
       // tel: [{type: ["work"], value: "123456", carrier: "testCarrier"} , {type: ["home", "fax"], value: "+55 (31) 9876-3456"}],
       is(findResult1.tel[0].carrier, "testCarrier", "Same Carrier");
       is(findResult1.tel[0].type, "work", "Same type");
@@ -1285,30 +1285,30 @@ var steps = [
       ok(true, "Deleted the database");
       next();
     }
     req.onerror = onFailure;
   },
   function () {
     ok(true, "Adding empty contact");
     createResult1 = new mozContact();
-    createResult1.init({givenName: 5});
+    createResult1.init({givenName: "5"});
     req = navigator.mozContacts.save(createResult1);
     req.onsuccess = function () {
       ok(createResult1.id, "The contact now has an ID.");
       sample_id1 = createResult1.id;
       next();
     };
     req.onerror = onFailure;
   },
   function () {
     ok(true, "Test category search with equals");
     var options = {filterBy: ["givenName"],
                    filterOp: "contains",
-                   filterValue: 5};
+                   filterValue: "5"};
     req = mozContacts.find(options);
     req.onsuccess = function () {
       ok(req.result.length == 1, "1 Entry.");
       checkContacts(req.result[0], createResult1);
       next();
     }
     req.onerror = onFailure;
   },
@@ -1317,16 +1317,71 @@ var steps = [
     req = mozContacts.clear()
     req.onsuccess = function () {
       ok(true, "Deleted the database");
       next();
     }
     req.onerror = onFailure;
   },
   function () {
+    ok(true, "Adding contact with invalid data");
+    var input = document.createElement("input");
+    var obj = {
+        name: [1, 2],
+        familyName: 3,
+        givenName: 4,
+        honorificPrefix: [],
+        honorificSuffix: {foo: "bar"},
+        additionalName: 7,
+        nickname: [8, 9],
+        org: [10, 11],
+        jobTitle: [12, 13],
+        note: 14,
+        category: [15, 16],
+        sex: 17,
+        genderIdentity: 18,
+        email: input,
+        adr: input,
+        tel: input,
+        impp: input,
+        url: input
+    };
+    obj.honorificPrefix.__defineGetter__('0',(function() {
+      var c = 0;
+      return function() {
+        if (c == 0) {
+          c++;
+          return "string";
+        } else {
+          return {foo:"bar"};
+        }
+      }
+    })());
+    createResult1 = new mozContact();
+    createResult1.init(obj);
+    req = mozContacts.save(createResult1);
+    req.onsuccess = function () {
+      checkContacts(createResult1, {
+        honorificPrefix: "string",
+        sex: "17",
+        genderIdentity: "18"
+      });
+      next();
+    };
+  },
+  function () {
+    ok(true, "Deleting database");
+    req = mozContacts.clear()
+    req.onsuccess = function () {
+      ok(true, "Deleted the database");
+      next();
+    }
+    req.onerror = onFailure;
+  },
+  function () {
     ok(true, "all done!\n");
     clearTemps();
 
     SimpleTest.finish();
   }
 ];
 
 function next() {
--- a/dom/interfaces/contacts/nsIDOMContactProperties.idl
+++ b/dom/interfaces/contacts/nsIDOMContactProperties.idl
@@ -42,17 +42,17 @@ interface nsIDOMContactFindSortOptions :
 interface nsIDOMContactFindOptions : nsIDOMContactFindSortOptions
 {
   attribute DOMString filterValue;  // e.g. "Tom"
   attribute DOMString filterOp;     // e.g. "contains"
   attribute jsval filterBy;         // DOMString[], e.g. ["givenName", "nickname"]
   attribute unsigned long filterLimit;
 };
 
-[scriptable, uuid(f0ddb360-e4aa-11e1-9b23-0800200c9a66)]
+[scriptable, uuid(86504745-da3e-4a17-b67b-88b40e0e6bc8)]
 interface nsIDOMContactProperties : nsISupports
 {
   attribute jsval         name;               // DOMString[]
   attribute jsval         honorificPrefix;    // DOMString[]
   attribute jsval         givenName;          // DOMString[]
   attribute jsval         additionalName;     // DOMString[]
   attribute jsval         familyName;         // DOMString[]
   attribute jsval         honorificSuffix;    // DOMString[]
@@ -64,11 +64,11 @@ interface nsIDOMContactProperties : nsIS
   attribute jsval         adr;                // ContactAddress[]
   attribute jsval         tel;                // ContactTelField[]
   attribute jsval         org;                // DOMString[]
   attribute jsval         jobTitle;           // DOMString[]
   attribute jsval         bday;               // Date
   attribute jsval         note;               // DOMString[]
   attribute jsval         impp;               // ContactField[]
   attribute jsval         anniversary;        // Date
-  attribute jsval         sex;                // DOMString
-  attribute jsval         genderIdentity;     // DOMString
+  attribute DOMString     sex;
+  attribute DOMString     genderIdentity;
 };