Rails 一个小 Bug 引发 github 安全问题泄露。Shit!
如果你去了 v2ex，一定会看到这个标题：Rails 爆出严重安全漏洞 (http://www.v2ex.com/t/28768#reply2), 别激动，v2ex 对 ruby 来说，它是外行社区，看了帖子内容我就意识到都是在看热闹，没说道点子上。
Egor Homakov，这个哥们是一个 Rails 开发者。报出的问题是 mass assignment 没保护的问题。这个问题是什么：看这个： Hackers Love Mass Assignment #26 Hackers Love Mass Assignment（http://railscasts.com/episodes/26-hackers-love-mass-assignment ）
github 发现后，把这个 Homakov 的账号给 ban 住了。这种反映让人不爽，（ http://chrisacky.posterous.com/github-you-have-let-us-all-down ），在危机公关上是个败笔。
最后，好消息，@wycats已经提议强制保护（https://gist.github.com/1974187 ） 后又写了他对 attr_accessible (and an analogy to rails_xss ) 的安全观点（http://news.ycombinator.com/item?id=3664334 ），很快已经有人就把代码写到 rails-master 分支了（https://github.com/rails/rails/commit/641a4f62405cc2765424320932902ed8076b5d38 ）
@dhh 老大发话了，请大家记录之： Here's the basic pattern we've been using at 37signals to guard against mass-assignment bugs: https://gist.github.com/1975644
class PostsController < ActionController::Base
def create
Post.create(post_params)
end
def update
Post.find(params[:id]).update_attributes!(post_params)
end
private
def post_params
params[:post].slice(:title, :content)
end
end
对，这是 Github 里写这段代码的人代码写差了，这里用attr_accessible也能起到该有的作用。这和 Rails 没有半毛钱关系，用任何的工具都能写出这种危险的代码。
之前 Github 还有个信用卡信息没有校验的 Bug，可以绕过信用卡信息随意更改 Plan。
原来是 attr_accessible, attr_protected 的关系 http://huacnlee.com/blog/rails-attr-protected-or-attr-accesible-fields/
#5 楼 @huacnlee 我在 Django 里模仿 Rails 写了个 mass-assignment 的东西，现在有点担心会不会有漏洞 = =
class BaseModel(db.Model):
protected_fields = []
def to_dict(self):
keys = self.fields().keys()
_dict = {}
for key in keys:
_dict[key] = getattr(self, key)
return _dict
def from_dict(self, _dict):
for field in self.protected_fields:
if field in _dict:
del _dict[field]
for key, value in _dict.iteritems():
setattr(self, key, value)
self.put()
update_attributes，我刚开始学 rails 的时候就想过这个问题，应该会引起安全问题，尤其是对于有 password 的 user 表，不过看了一下，发现 rails 本身已经有解决办法，attr_protected。
从 rails 的 guide 有一章安全来看，rails 本身对安全是很重视的，至少比其他的 web 框架考虑得多，包括 xss,csrf。在 django 中我是没看过安全这章。
这里扯上 rails，说是其 bug 真的太扯了，是 github 用得不对。
我觉得用 attr_accessible 会比较好些，不然经常会忘记，比如这里 updated_at 和 created_at 就会被 mass assigned.. 虽然是不会照成什么很严重的后果。。
我觉得他是因为这个 issue( https://github.com/rails/rails/issues/5228 ) 没被 rails 采纳。。才试图用 hack github 的方式让开发者重视，或是让 rails 采纳他的建议....
我觉得 fxn 说的很有道理，这是开发者的问题，而不是 rails 的问题，事实上 rails 已经提供了很好的解决办法，开发者没有意识到是自己的问题。
昨天还去 ruby china 看了一下，，发现也有类似问题：https://github.com/huacnlee/ruby-china/blob/master/app/controllers/topics_controller.rb#L94
貌似我用了 Rails 那么久一直都是用 attr_accessible，没有用过 attr_protected（我是看 Ruby on Rails Tutorial 这本书入门的，对我的编码风格影响很大）
我觉得 homakov 的初衷是让 rails 团队提供一个安全的默认方案。目前的默认方案是不安全的。
一楼 dhh 自己的解决方案也从侧面反映出当前 attr_accessible 的方案并不是最优。
Yehuda 的 proposal 在这里：https://gist.github.com/1974187
我个人非常赞同 safe guarding 应该是 controller 的责任，而不是 model 的责任。
37signals 那樣作是有原因的。
- 把責任移到 controller
- attr_accessible 不是最好的方案。很容易搞死人，而且有時候沒有解決到問題...
我觉得白名单写进 Model 好，不然控制器还得知道 Model 的属性。用了 attr_accessible 之后，控制器知道 role 就行了，相当于打包属性组。
@homakov竟然是 93 年的，真是后生可畏啊。
Salut, I am Egor Homakov.
born on 28 apr. 1993 in Russia.
#23 楼 @hisea dhh 说他喜欢这种'hack'的原因是因为The great thing about post_params is that you can make it dependent on controller state as well
其实我觉得attr_accessor+as完全可以实现相同效果，而且还很 dry。
#32 楼 @Rei 全部白名单和不写都一样，只是能做个提醒，还真得怪 github 的程序员偷懒。
Role based mass assignment protection:
class Post < ActiveRecord::Base
attr_accessible :title, :body
attr_accessible :title, :body, :user_id, :rank, :as => :admin
end
Post.update_attributes(params[:post], :as => :admin)
哪个角色可以操作 model 的哪些属性难道不是业务逻辑？写到 controller 里我还真理解不了:-)
我觉得 arre_accessible 无需想到什么谁的责任什么的问题，arre_accessible 正是对对象属性的一种描述，controller 的责任不是操作 model 或者谁，而是接收业务请求跟着 role 走
@fredwu 验证数据作为业务逻辑的一部分，当然是 model 的事情. 而且，也许不止 PostsController 一个地方会更新 Post，写入 model，其他地方也一样调用。不然，若是某天允许修改 Post 的其他属性，要改无数个地方。
不同的 controller 可能需要不同的 attr_accessible, 我想这也是@fredwu说操作 model 是 controller 的责任。
如果只给最少的权限，controller 中将会有大量的。
@post.att1=123 @post.att2=234 ... ....
又是一个典型的易用性和安全性的冲突问题，说实话，虽然这个不能称之为 rails 的 bug，但是 rails 在这上面的处理确实不妥，导致不少“中低”水平的开发者做出的应用暗藏隐患 另外参考 rails3 对于 xss 的处理方式，我觉得 rails 确实需要在 mass assign 上做一些工作，safe by default 对框架来说还是很重要的
把 AngelNest 的 mass assignment 漏洞给修补了，嘿嘿：https://github.com/fredwu/angel_nest/commit/ab636d677295cca9c42e525bc737dc6978e1a663
其实目前 attr_accessible 这种方式最大的问题是容易被忘掉。哪怕你再牛逼。程序员是“懒惰”的，或者偶尔失误的。 一有这种状况，这种漏洞必然会出现。
文档里（attr_accessible，attr_protected）豁然写着一句
“Note that using Hash#except or Hash#slice in place of attr_accessible to sanitize attributes won’t provide sufficient protection.”
我可以肯定这句文档不是 DHH 写的....甚至没有看到过，不然他肯定会抹掉.... lol...
对的，需要根据不同的 controller 设置不同的限制。内部也有无数需要操作 model 的地方，限制写在 model 里面不妥。
针对这个问题改了一上午的代码，下次写 rails 的时候就注意到这个问题了。
#52 楼 @Rei 是的，我同意你这个观点，而且复杂的逻辑在 Model 里面更容易测试。这也是我不喜欢 dhh 的 controller 私有方法的原因。
同时，我也同意@fredwu 跟 yahuda 的观点，我论是 role 也好，还是 current_user 的判断也好，authentication 和 authrization 是 controller 的责任。如果我们不在 model 里面判断用户是否登陆的话，为什么吧不同 role 可以更新哪些 field 的声明牵到 model 中来呢。
可能很多人会觉得登录和 mass assignment 不能比较，因为登录是个相对简单很多的问题。
我个人觉得简单是因为登陆已经成为成熟的模式，更重要的是登录有很好的工具完成这一切。而 mass assignment 处于混沌状态，需要人一斧子劈开混沌找一个成熟的模式，并开发一个易用的工具。我想 Yahuda 发一个 Proposal 而不是直接写代码添加到 rails 里面，也说明 rails 团队同样也是在摸索，希望得到开发人员的反馈。
我只是觉得现有的方式无论是 attr_accessible 或者是 controller hack 都不是最佳方案，虽然我暂时也没想出什么好办法，毕竟 mass assignment 其实涉及到的很多 rails 内部机制，是 rails 提高开发速度的 magic 之一，限制多了魔法受限，限制少了走火入魔。
我提几个我认为关键的问题：
- mass assignment 这个问题上，model 是否应该信任 controller。
- dhh 的做法和 model 里验证的做法是否冲突？ 可不可以把 dhh 的做法理解为，filter 掉 params 里不需要的值。 如果这么理解，dhh 的做法是无可厚非的。
- 把 user,role 等和当前 session 相关的东西传给 model，是不是一种变相的让 model 知道当前的状态？
#63 楼 @hooopo validation 是 validate model 自己内部的状态。不取决于外部的条件。 不取决于用户角色，比如 post title 不能为空，无论谁登录都不能为空，无论是 web 登录还是 api 发布都不能为空，即便是 console，rake task, seed import，没有 controller，没有 session 保存依然不能为空。这个条件是不为外部条件所控制的。这种自足 (self contained) 的模式有利于 model 不依赖外界条件的情况下证明自己的合理性。同时降低了耦合，哪天我们的世界没有了 web，没有浏览器，没有了 session 和 controller，这个 model 依然可一作为一个独立的个体，在不改变代码的情况下继续工作。
validation 可以有判断，但这个判断也应该是内部的，比如 post 的 title 在 slug 为空的情况下不能为空，slug 同样是 post 内部的状态。
反过来说，很少有人 validation 里面是根据其他不相关的外部因素来判断的，例如很少 validation 牵扯到 role 或者 current_user。为甚么不能在 validation 中加入 current_user 的判断呢，看传进来的一个 flag，确定是不是登录用户，或者是不是 admin，admin 就能 save，不然 validation error 就是，"你保存权限不够"，这样不也说得过去么。
换一个稍微远一点例子易说明，就是 callback 跟 observer.
我个人的原则就是 callback 用与自身内部的事务，observer 牵扯外部的事务。
比如，如果 slug 为空，保存之前将 title 复制到 slug，都是内部的事情，用 before_save 很不错，用 observer 有点多此一举。
反过来，如果保存之后需要发一封邮件，访问一个第三方 api，然后更新服务器硬盘上的一个文件，这些东西放在 after_save 里面就怪怪的了。
#64 楼 @poshboytl 个人认为 1、mass assignment 必须信任 controller 2、dhh 的做法对于 3.1+ 是多此一举，mass assignment 是 3.1 加入的功能，3.1 以前确实按 dhh 那样干过 3、给 model 只需要传 user_id 就好，不需要 session 啊。
另外对于 user_id 我的做法是，加个 set_current_user before_filter，这样 request 过来的 user_id 都会被覆盖。model 里不对 user_id 做权限方面的验证，只管保存就好，是不是 admin 都在 controller 里判断
private
def set_current_user(resource_name=nil, attribute_name="user_id")
return unless current_user.respond_to?(:id)
resource_name ||= controller_name.singularize
params[resource_name] ||= {}
params[resource_name][attribute_name] = current_user.id
end
attr_accessible :title, :body, :as => [:default, :admin]
attr_accessible :user_id, :rank, :as => :admin
Strong parameters: Dealing with mass assignment in the controller instead of the model
