form_prep security issue when using arrays #228

Closed
kenjis opened this Issue Aug 21, 2011 · 2 comments

Projects

None yet

3 participants

@kenjis

Since form_prep check the field being prepared:

if (isset($prepped_fields[$field_name])) { return $str; }

When using arrays of post data like

foreach($item in $items) echo form_input('item[]', $item)

After the first item nothing gets escaped. The comment says this is a todo, but still is a problem.

from: https://bitbucket.org/ellislab/codeigniter/issue/365/form_prep-security-issue-when-using-arrays

@sourcejedi

Interesting. I interpreted the TODO as talking about fields with the same name in different forms. But I forgot individual HTML forms are expected to have multiple copies of the same name, e.g. it's how radio buttons work.

I would just kill $prepped_fields. It's not possible to implement usefully. It looks like an attempt at optimization, but I can't see any justification for it.

Then form_prep() would be deprecated, because all that would be left is a call to html_escape(). (html_escape() can already handle arrays).

@narfbg narfbg added a commit that closed this issue Oct 26, 2012
@narfbg narfbg Deprecated form helper function form_prep().
This function has been broken for YEARS and it's value-caching
logic has only introduced various problems. We have html_escape()
since CI 2.1.0 which is a perfect replacement, so it should be
used instead.

Fixes #228 & #1630
74ffd17
@narfbg narfbg closed this in 74ffd17 Oct 26, 2012
@nonchip nonchip pushed a commit to nonchip/CodeIgniter that referenced this issue Jun 29, 2013
@narfbg narfbg Deprecated form helper function form_prep().
This function has been broken for YEARS and it's value-caching
logic has only introduced various problems. We have html_escape()
since CI 2.1.0 which is a perfect replacement, so it should be
used instead.

Fixes #228 & #1630
8ed4f77
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment