0

So our ancient web app manages to pass an annual code review with minimal red flags. One of those possible security risks was this function below that gets a user's session info.

private function getJobTrackerSessionInfo()
{
    // Get session data from database
    $query ='SELECT `a_session` FROM `sessions` WHERE id = ?';
    $a_session = $this->db->getOne($query,null,array(session_id()));

    //not logged in returns false
    if(empty($a_session)) return array();

    //convert session stuff from the database into a php array
    $a_session = str_replace("{","array(",$a_session);
    $a_session = str_replace("}",")",$a_session);
    $a_session = str_replace(";\$D","",$a_session);
    @eval($a_session);

    return $D;
}

This private function call is done after a CSRF token verification. I'm new to PHP and SQL database calls. I can understand it, but as far as writing securely, I'm out of my element. This is currently a web app running in like PHP 5, but we do plan on bringing it past 7+ by the summer.

I was reading through this Stackoverflow thread here: How can I prevent SQL injection in PHP?

It may be dated, but the user replies below had a lot of useful information on depreciated and up to date preventative methods.

Is there a good updated solution to prevent any sort of injection during this specific query and eval?

Edit: Removing the eval() function call seems to be what could help secure this more, but the rabbit hole goes deeper. getOne() is supposed to be called and populate a_session. However, I can't seem to find where the heck the getOne() function lives. I did find that it belongs to some sort of sql dev api that can be reviewed here: https://www.php.net/manual/en/mysql-xdevapi-collection.getone.php

Edit 2: From dumping $a_session, I found that it is code written to apply an array of values to $D as a string meant to execute in eval() Okay, that seems obvious, but it isn't just containing $_SESSION info parameters, but also user and token info. I guess the author's original thought process was to maybe keep those sensitive user values hidden when assigning and returning them?

Could I just parse the string, assign array(a bunch of values) to a value, and just do $D = $myParsedArray. $myParsedArray would still be interpreted as a string though, right?

I need to think on this one. I'll post updates with any progress.

Improve this question
13
  • 2
    "Is there a good updated solution to prevent any sort of injection" Yeah, don't use eval(). Commented Jan 26, 2022 at 16:30
  • How does the string $a_session looks like from $this->db->getOne? Commented Jan 26, 2022 at 16:31
  • or create a new modified function that does the same as that Commented Jan 26, 2022 at 16:32
  • 1
    Your .getOne() method must be declared in a database class in your application. The .getOne() method you use has different parameters from the one in php's mysqlx API to MySQL's document store functionality. At any rate it looks like your .getOne() uses bind parameters. That's the right way to avoid SQL injection. But eval() is still very dangerous indeed, as it runs whatever code you give it. Commented Jan 26, 2022 at 17:16
  • 1
    You can't have an instance of a function, it has to be an instance of a class. The db_connect() function instantiates a new object and returns it. What type of object? Commented Jan 26, 2022 at 17:19
Related questions
24
Best way to avoid code injection in PHP
8
How do you prevent SQL injection in LAMP applications?
15
Best way to defend against mysql injection and cross site scripting
Load 6 more related questions Show fewer related questions

0

Reset to default

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.