Fix security issue with sequelize generated id attribute

changelog.md

@@ -8,6 +8,7 @@ Notice: All 1.7.x changes are present in 2.0.x aswell
 - [Feature] Support for HAVING queries. [#1286](https://github.com/sequelize/sequelize/pull/1286)
 - bulkUpdate and bulkDestroy now returns affected rows. [#1293](https://github.com/sequelize/sequelize/pull/1293)
 - fixes transaction memory leak issue
+- fixes security issue where it was possible to overwrite the id attribute when defined by sequelize (screwup - and fix - by mickhansen)
 
 # v1.7.0-rc2
 - fixes unixSocket connections for mariadb [#1248](https://github.com/sequelize/sequelize/pull/1248)

lib/dao-factory.js

@@ -164,7 +164,7 @@ module.exports = (function() {
 
     this.DAO.prototype._hasPrimaryKeys = this.options.hasPrimaryKeys
     this.DAO.prototype._isPrimaryKey = Utils._.memoize(function (key) {
-      return self.primaryKeyAttributes.indexOf(key) !== -1
+      return self.primaryKeyAttributes.indexOf(key) !== -1 && key !== 'id'
     })
 
     if (this.options.timestamps) {

lib/dao.js

@@ -170,6 +170,12 @@ module.exports = (function() {
             return
           }
 
+          // If attempting to set generated id and id is already defined, return
+          // This is hack since generated id is not in primaryKeys, although it should be
+          if (originalValue && key === "id") {
+            return
+          }
+
           // If attempting to set read only attributes, return
           if (!options.raw && this._hasReadOnlyAttributes && this._isReadOnlyAttribute(key)) {
             return
@@ -485,14 +491,13 @@ module.exports = (function() {
     return validator.hookValidate()
   }
 
-  DAO.prototype.updateAttributes = function(updates, fieldsOrOptions) {
-    if (fieldsOrOptions instanceof Array) {
-      fieldsOrOptions = { fields: fieldsOrOptions }
+  DAO.prototype.updateAttributes = function(updates, options) {
+    if (options instanceof Array) {
+      options = { fields: options }
     }
 
-    this.setAttributes(updates)
-
-    return this.save(fieldsOrOptions)
+    this.set(updates)
+    return this.save(options)
   }
 
   DAO.prototype.setAttributes = function(updates) {

test/dao/values.test.js

@@ -17,7 +17,24 @@ chai.Assertion.includeStack = true
 describe(Support.getTestDialectTeaser("DAO"), function () {
   describe('Values', function () {
     describe('set', function () {
-      it('doesn\'t overwrite primary keys', function () {
+      it('doesn\'t overwrite generated primary keys', function () {
+        var User = this.sequelize.define('User', {
+          name: {type: DataTypes.STRING}
+        })
+
+        var user = User.build({id: 1, name: 'Mick'})
+
+        expect(user.get('id')).to.equal(1)
+        expect(user.get('name')).to.equal('Mick')
+        user.set({
+          id: 2,
+          name: 'Jan'
+        })
+        expect(user.get('id')).to.equal(1)
+        expect(user.get('name')).to.equal('Jan')
+      })
+
+      it('doesn\'t overwrite defined primary keys', function () {
         var User = this.sequelize.define('User', {
           identifier: {type: DataTypes.STRING, primaryKey: true}
         })