Vulnerable bug in CodeIgniter which took us hours to fix our corrupted database

18 thoughts on “Vulnerable bug in CodeIgniter which took us hours to fix our corrupted database”

  1. sorry hasan, I am a Big CI Fan. I dont want to f**k it just for this small bug, they themselves will fix it.

    CI Rocks. It makes my life a lot easier than before.

    Regards

  2. Pingback: PHPDeveloper.org
  3. “You found a bug, you reported it, the code is being worked on. No need to blab all over the word about your “bad experience” with CI. Especially when the problem was partially yours for not checking your data. Methinks your ego must be gigantic.” — JAAulde

  4. For the record, I am “JAAulde” from the CI forums (among places) and I want you to know that while I did originate the above quote in your thread at CI, I did not cross-post it to your blog.

    Jim (JAAulde)

  5. I am a CI fan, at the first, this small problem doesn’t append if you validate the form with form validation class, it is absolutely necessary validate the user input ( in the client side, and ALLWAYS in the server side ) before database modification, in other case you run with the risk of database corruption 😉 . Of course, if your project is a commercial project, this care is more important.

  6. Hi Sevir, i agree that I must the user input beofre sending it to CI, if you check the forum post also there was a huge discussion that as I sent ti a null value and CI didn’t interpret it correctly.

    If I send a “null” to CI, ActiveRecord must not apply any Fuzilogic to it and it should be treated as null, what CI developers overlooked.

  7. While I’ll start by saying that use, this does appear to be a bug in the ActiveRecord class used by CI, there are ways to avoid this from happening.

    First off, this issue should have never made an impact on you, other than finding the bug and fixing it. It should have never affected your users. You should have made your code change in a development environment, and tested them as a developer. Then pushed those changes to a staging site, for user acceptance testing with multiple users who are not developers, and then once your confirm the changes were valid, made the changes live. This should be your standard development cycle, and never be circumvented for even the tiniest changes, to avoid this from happening.

    Secondly, you may want to examine your data validation practices. Most websites/applications are multi-tiered. You have the user interface, the backend, and the database. You validation practices should look like

    1. (optional) Interface validation. Speaking of websites, this would be javascript to validate form entries before allowing the submission. This is just a little nicer than kicking responses from the backend, as it cuts down client to server communication.

    2. Backend data validation. Even if you validate at the interface, validate the data when it reaches the server. You have no way of guaranteeing where the data comes from.

    3. When passing from your script/run time engine to the database, validate again. Just to avoid cases like these. While much of your data has come direct from the source, things like your user id may have been generated as part of the algorithm creating the query. So validate the data going into the query. Though I imagine you were counting on ActiveRecord for this, and honestly, I would too.

    Lastly, you may to consider how you put your words together when putting together blog entries such as these. Honestly, I was put off by your colorful language. I’m a sysadmin with over 10 years of experience, so believe me, I fully appreciate the level of frustration you get when you have situations like these. However, the way you wrote this post is much better suited for behind closed door conversations and letting out steam with your co-workers, not for the world (and possible future potential employers) to see. Just a thought.

    Good luck with your future projects.

  8. This is really too bad, because I asked a question about this exact same issue last year in the CodeIgniter forum. (well, close enough, but I was concerned with passing in a “false” instead of a null). All I got was a single rude response from a community member questioning my knowledge of PHP.

    http://codeigniter.com/forums/viewthread/47012/

    I really hope this behavior changes so that it doesn’t cause problems for anyone else in the future.

  9. @Bowman, I do not work with him, but I do read his posts, because it helps me a lot. This post is really helpful, coz nowadays, I am building lots of models in CodeIgniters, and after now, I would be careful, while I am gonna run any query using the Active Record Class. He may not be native english speaker, but he is technically strong enough to grab the attention of almost all the php developers from Bangladesh.

    @hasin bhai, I would be careful. Thanks for this post.

  10. well, as most developers will do before processing data is to first and foremost, validate entries. right? it’s a simple logic..

  11. 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.

  12. humm… it is always good to have someone to blame for our incompetence.
    This is not a bug of CI.
    You must go with some basic “sanity checks” on data when you are using a database, if you are serious about development, of course.

  13. Hello Hasin vai,

    This bug is not all, codeigniter has a lot more weaknesses, like their input library doesn’t filter all malicious html tags. And also when using form validation with file uploads it won’t let u validate the file fields. And also if you want use form validation with modules it will give u some trouble. There are currently many layout library for codeigniter. but they only support one single block in the layout. that’s a problem.

    I have created extended libraries for minimizing all those bugs. For every one interested, you can find those tahSin’s gaRAge. thanks.

Leave a comment