Bad Experience With CodeIgniter

Written by on April 19, 2007 in General - 12 Comments
codeigniter.gif

Hasin Hayder, shared with us his bad experience with CodeIgniter Open Source Framework. I understand the troubles that happened to their users database with such unexpected behavior, but it’s something to learn from. Personally I will never allow updating my tables without a clear and correct where condition. Below his email :

We use CodeIgniter internally to develop our web solutions. Day before yesterday we suffered a terrible situation for an internal bug in code igniter which corrupted data inside some tables of our application database and then it took hours to find the origin of that bug, to fix it and to repair the corrupted data. Let me explain what happened.

Lets guess that we have one table named “users” with the following field

1. id
2. username
3. password
4. email

At some point, if you want to update the password field of this table, for a particular user, what will you write in your code?

CodeIgniter’s ORM will create the query like the following

UPDATE users set password='{$new_password}' where user_id='{$user_id}';

Well, it’s ok and the query seems pretty fine. Now what should happen if you pass a valid user id to this code? Password of only that user will be updated. But what will happen when the passed $user_id is null?? Thats the most pathetic part that Codeigniter ORM plays. Instead of generating the following query,

UPDATE users set password='{$new_password}' where user_id='';

CodeIgniter’s ORM actually generates the following

UPDATE users set password='{$new_password}' where user_id;

You find the difference of the above two queries right? one contains “where user_id=” ” and another contains just “where user_id” . Now if your backend database is MySQL and this query executes? You know what the hell will happen? It will replace all the user’s password with this new password instead of failing as MySQL count the “where user_id” part equals to false and returns all users. But If your Database is PostgreSQL, it fails, you are lucky.

So day before yesterday we suffered this problem against our commercial application which corrupts our user profile data. We immediate fixed the issue from our backup db (well, we lost 3 data) and then we started to find out what actually went wrong and found this vulnerable bug in CI.

So we suggest the CodeIgniter team to fix the issue immediately and change their ORM code so that it creates the query like the following if the value of passed argument is null. because it will fail to execute in all db. Otherwise the fellow user’s of code igniter, prepare for the dooms day.

UPDATE users set password='{$new_password}' where user_id='';

12 Comments on "Bad Experience With CodeIgniter"

  1. Hasin Hayder April 19, 2007 at 3:34 pm · Reply

    This things happen while developing a web solution for somewhere in net limited (www.somewherein.net/blog).

  2. Hasin Hayder April 19, 2007 at 3:45 pm · Reply

    Please dont mix up this article with Pageflakes.

  3. Hatem April 19, 2007 at 3:54 pm · Reply

    Thanks Hasin, I have updated the post. Anyway I know that Pageflakes is powered by AJAX ASP.NET.

  4. Hasin Hayder April 19, 2007 at 4:09 pm · Reply

    Thanks Hatem, A lot.

  5. Dan August 27, 2007 at 10:07 am · Reply

    Your too stupid to try it on a live database and too assuming not to give a default value (like -1) if userid is null. I think it’s all the bug in your head not in the CI itself.

  6. Michael Wales November 19, 2007 at 1:27 pm · Reply

    This has been mentioned before and is a very well-known fact within the CI community. I do believe EllisLabs is intending on changing the functionality (so this doesn’t happen) in future versions, but there is a strong case for validating your input.
    Based on the code pasted here – please give us your URL so we can all create admin accounts.
    I would highly suggest you backtrack through all of your queries and ensure you are receiving the correct data, from the correct location, in the correct format. If this is done properly – is it literally impossible for this bug to affect you.

  7. Oggers May 5, 2008 at 12:41 pm · Reply

    Always better to type cast when using IDs, I usually do $this->db->where(“id”, (int)$user_id);

  8. gillbates June 18, 2008 at 10:52 am · Reply

    (int)null is 0.
    Might still cause unpredictable behavior.

  9. supergahar February 13, 2009 at 11:53 pm · Reply

    the fact that NULL $user_id can go through that line of code is beyond me. Oh well, it is the days of scripting coder

  10. Donny Kurnia June 27, 2009 at 2:32 pm · Reply

    Bad programming habit, but blame the framework :)
    Next time, write model with default value for each parameter, then check it again before using it in a query.

  11. dizzystuff July 10, 2009 at 2:13 pm · Reply

    Pretty certain that if you’ve not validated the input by the time you’re running the update on your db… the problem is a ‘vulnerable bug’ in your coding not the tools. IMHO. Sitting here a bit dumbfounded by the disbelief expressed in his email.

  12. Davide November 19, 2009 at 10:55 pm · Reply

    This is why i prefer code SQL query by hand than leave all to ORM. Ok you lose reliability but earn full controll over each query. IMO.

Leave a Comment