Hey, since Jack quit, you could add Jane to the investigation users?

William M supported one piece of a large, complicated intranet application used by auditors as part of investigations into events within the organization. The overall audience is small, and the purpose of the application is mostly to document the process of the investigation. Since its so niche, it doesnt even have a screen for managing user accounts- the users raise a ticket and rope in developers to manage it.

The developer in charge of the secure portion of the application- Rick- was busy, so William caught the ticket. He wasnt sure what to do, so he shot an IM over to Rick.

Oh, its easy, Rick replied. Just update Jacks record with Janes user info. Thats the easiest way, since Jack quit.

That didnt seem like a smart process, but William was in a rush and this was Ricks baby, so he just followed instructions and closed the ticket. Jane re-opened the ticket- she couldnt access the audit compliance tracker. Jack had never used that page, but Jane planned to.

Rick was still busy, so William decided to take a look at the authorization process. He found this code in the main-menu screen of the application.

if (Convert.ToInt32(Session["UserID"]) == 201 ||
        Convert.ToInt32(Session["UserID"]) == 226 ||
        Convert.ToInt32(Session["UserID"]) == 214 ||
        Convert.ToInt32(Session["UserID"]) == 258 ||
        Convert.ToInt32(Session["UserID"]) == 260 ||
        Convert.ToInt32(Session["UserID"]) == 22 ||
        Convert.ToInt32(Session["UserID"]) == 303 ||
        Convert.ToInt32(Session["UserID"]) == 444 ||
        Convert.ToInt32(Session["UserID"]) == 32 ||
        Convert.ToInt32(Session["UserID"]) == 388 ||
        Convert.ToInt32(Session["UserID"]) == 413 ||
        Convert.ToInt32(Session["UserID"]) == 392)
{
    btnSecureArea.Visible = true;
}

No… no way.

William checked one of the other secure pages.

int userID;
if (!int.TryParse(Session["UserID"].ToString(), out userID))
{
    Response.Redirect("NavigationPage.aspx");
}
if (
    !(userID == 201  //Jerry
    || userID == 226 //Lisa
    || userID == 214 //Alban
    || userID == 258 //David
    || userID == 22  //Jack
    || userID == 444 //Tyrell
    || userID == 303 //Brenda
    || userID == 413 //Phung
    || userID == 388 //Ignacio
    || userID == 399)//Norma
    )
{
    Response.Redirect("NavigationPage.aspx");
}

This wasnt looking so great. William pulled up the code for the audit compliance tracker that was causing Jane trouble.

if (Convert.ToInt32(Session["UserID"]) == 201 ||//Jerry
    Convert.ToInt32(Session["UserID"]) == 226 ||//Lisa
    Convert.ToInt32(Session["UserID"]) == 214 ||//Alban
    Convert.ToInt32(Session["UserID"]) == 258 ||//David
    Convert.ToInt32(Session["UserID"]) == 303 ||//Brenda
    Convert.ToInt32(Session["UserID"]) == 32  ||//Clementina
    Convert.ToInt32(Session["UserID"]) == 199 ||//Shyla
    Convert.ToInt32(Session["UserID"]) == 232 ||//Harmony
    Convert.ToInt32(Session["UserID"]) == 413 ||//Phung
    Convert.ToInt32(Session["UserID"]) == 388 ||//Ignacio
    Convert.ToInt32(Session["UserID"]) == 399)  //Norma
{
}
else
{
    Response.Redirect("NavigationPage.aspx");
}

Well, it wasnt hard to see why Jack-now-Janes user id got kicked out of the page. With all the copypasta, things had gotten out of sync. Even better, this code was inside of the Page_Load handler on a WebForm, and since it didnt return on failure, the rest of the Page_Load event executed, even though the user didnt have permission to access the page.

William didnt have the time or permission to actually move this data into the database, but he did spend the time to refactor the copy/pasted code into a single method that used a list of IDs. By making sure the authorizer quits on failure, he also fixed half-a-dozen low-priority unexplainable bugs caused by side effects from the Page_Load executing.

[Advertisement] Scout is the best way to monitor your critical server infrastructure. With over 90 open source plugins, robust alerting, beautiful dashboards and a 5 minute install - Scout saves youvaluable engineering time. Try the server monitoring you'll M today.Your first 30 days are free on us. Learn more at Scout.

http://thedailywtf.com/articles/authorized-users