Looks like cattr_accessors, defined for ActiveRecord::Base can be set
through mass assignment of attributes for new model instances.
Following example uses table "model (id serial, name text)", and with
script/console :
class Model < ActiveRecord::Base; end
=> nil
ActiveRecord::Base.allow_concurrency
=> false
m = Model.new('allow_concurrency' => 'true', 'colorize_logging' =>
Ok, it's a hidden feature of all writer methods.
But not adding fields, defined as "cattr_accessor" in ActiveRecord::Base
to attributes_protected_by_default is, IMHO, ugly bug.
As using Model.create(params[:model]) or build(params[:item])
isn't (yet?) considered bad practice, and is, probably, quite widespread,
one can just add, for example, "comment[verification_timeout]=none" to
post parameters
and turn server to permanent error state, filling log with:
>
> It's a hidden feature of any writer method. Use attr_protected or
> attr_accessible to set which may be set through a mass assignment.
>
Ok, it's a hidden feature of all writer methods.
But not adding fields, defined as "cattr_accessor" in ActiveRecord::Base
to attributes_protected_by_default is, IMHO, ugly bug.
I agree, it's a ugly bug and a likely security issue in most Rails
apps. It would be better to have attributes_accessable_by_default
instead, defaulting to only the tables content_columns (except primary
key and magic columns).
The Scaffolding Extensions plugin isn't vulnerable to this because it
only update attributes that are shown on the form.
As using Model.create(params[:model]) or build(params[:item])
isn't (yet?) considered bad practice, and is, probably, quite widespread,
one can just add, for example, "comment[verification_timeout]=none" to
post parameters
and turn server to permanent error state, filling log with:
I have no patch, but I was about to write something to set
attr_accessible to a reasonable default to avoid explicitly writing
long lists of attributes.
I thought maybe I could share this code to collect ideas and opinions.
So I turned it in a plugin.
Looks like cattr_accessors, defined for ActiveRecord::Base can be set
through mass assignment of attributes for new model instances.
Following example uses table "model (id serial, name text)", and with
script/console :
class Model < ActiveRecord::Base; end
=> nil
ActiveRecord::Base.allow_concurrency
=> false
m = Model.new('allow_concurrency' => 'true', 'colorize_logging' =>
I wonder, if this is a bug, or just hidden feature of "cattr_accessor"?
It's a hidden feature of any writer method.
I'd have to agree that it's a bug -- at least, it's contrary to the
documentation:
AR::Base.new: ...valid attribute keys are determined by the column
names of the associated table — hence you can‘t have attributes
that aren‘t part of the table columns.
and, in general, "attributes" and "all the attributes" and such
phrases are used interchangeably in read and write contexts, without
suggesting that they're different.
I know that attr_accessible is available (and probably underutilized
by me and others), but based on the documentation, it sounds like even
the worst/loosest case should still not pull in the cattr values.
(I know the discussion has progressed to patch-hood, etc. -- I just
now was catching up though
This security issue was fixed in trunk with changesets 6050 and 6051.
However, neither of these has yet been merged to stable. Is this an
oversight, or was this intentional? If an oversight, can someone in
core merge the changes to 1.2-stable so that this security issue will
be fixed in 1.2.2? If intentional, can someone in core explain why?
