hemu13's avatar

Prevent Sql injection attacks

Here i have mentioned my query, this query can sql injection attacks possible

class AutoSuggestHelper {

/**
 * sendEmail
 *
 * Method uder for sending emails to users
 * 
 */
public static function autoSuggest($data)
{
    $term    = $data['q'];
    $type    = $data['type'];
    $results = array();
    switch ($type) {
        case 'chiefComplaint':
            $queries = DB::table('chief_complaints')
                    ->select('id', 'cc_text as value')
                    ->where('cc_text', 'LIKE', '%' . $term . '%')
                    ->take(15)->get();
            break;
        case 'questions':
            $queries = DB::table('users')
                    ->select('id', 'email as value')
                    ->where('email', 'LIKE', '%' . $term . '%')
                    ->where('user_role', '=', 'D')
                    ->where('is_account_active', '=', 'Y')
                    ->take(15)->get();
            break;
        case 'questionscategory':
            $queries = DB::table('patients')
                    ->select('id', 'email as value')
                    ->where('email', 'LIKE', '%' . $term . '%')
                    ->where('is_account_active', '=', 'Y')
                    ->take(15)->get();
            break;


        case 'userPhone':
            $queries = DB::table('patients')
                    //->select('id', 'contact_number as value')
                    ->select('id', DB::raw('TRIM(LEADING "+" FROM SUBSTRING_INDEX(`contact_number`, "-", -1)) as value'))
                    ->where('contact_number', 'LIKE', '%' . $term . '%')
                    ->take(15)->get();
            break;
    }
    foreach ($queries as $query) {
        $results[] = $query->value;
    }
    return $results;
}

}

0 likes
6 replies
jlrdw's avatar

Study and learn proper parameter binding.

Cronix's avatar

@jlrdw where is he improperly binding? If you actually look at the code you'll see everything is properly bound.

@hemu13 I don't see anything wrong with what you're doing. You're using everything properly. None of your raw statements are using user input and everything else is properly bound.

The only thing I see here is you don't need to select id in any of the queries since you never use it - you only use value.

Also instead of

foreach ($queries as $query) {
    $results[] = $query->value;
}

You could just

$results = $queries->pluck('value');
hemu13's avatar

In the header i have mentioned

namespace App\Helpers; use Illuminate\Support\Facades\DB;

I want to eloquent model here.

Thanks in advance.

Snapey's avatar

Another issue to consider, your code will crash if the $type does not match one of the four $type

Personally, I would use multiple if statements and return early in each with the data, or just build the query and perform the get() at the end.

Not sure what you mean by "I want to eloquent model here"

You want help changing this to use eloquent?

hemu13's avatar

You want help changing this to use eloquent?

Yes i want to change this to eloquent

Please or to participate in this conversation.