Article:
  Ten Security Checks for PHP, Part 2
Subject:   Avoid Loose Typing Intricacies - fix
Date:   2003-04-07 23:32:18
From:   ljweb
Rather than the
if ($user_id < 0 || $user_id > 2 || !isset($user_id)) {
line, I would advice the use of
if (is_int($user_id) && isset($users[$user_id])) {


The reason is, that even how small or large a usertable must be, there will always be someone skipping out from time to time, leaving empty slots in the list. Checking for larger/less-than is then not optimal and will produce yet another security flaw, as the subject of this very article states. Using "loginnames" rather than a number, would remove this problem same time.


I know, the code produces is not intended for production site (or I hope so), but the examples provided should rate as code worth copying and following.


/ Lars
www.ljweb.com

Full Threads Oldest First

Showing messages 1 through 1 of 1.

  • Avoid Loose Typing Intricacies - fix
    2003-04-08 00:58:01  clancymalcolm [View]

    The code is certainly not intended for a production site - the code was given as an example of what _NOT_ to do. The text discussing the code illustrates these flaws. It is based on the security hole found in PHPMyAdmin some time ago, but simplified for clarity and the ability to run as a stand-alone script. Your suggestion is certainly valid and follows the advice in the "Possible Fixes" section for this hole.

    If you are designing an application from scratch I would advise using a standard library such as the PEAR authentication library or the older PHPLib library - this way you get the benefit of code that is (hopefully) reviewed by lots of other developers and well tested. However, there are still many applications that use their own authentication mechanisms and these are frequently flawed. The example aims to illustrate a flawed implementation so that you are able to identify and fix such holes.

    Cheers,
    Clancy