amirsanni / Mini-Inventory-and-Sales-Management-System

An Inventory and Sales Management System written in PHP (codeIgniter) with support for MySQL and Sqlite3 databases
https://1410inc.xyz/mini-inventory-and-sales-management-system/
MIT License
500 stars 268 forks source link

SQL injection Vulnerability at latr_/?orderBy=transId #60

Closed lawrenceamer closed 5 years ago

lawrenceamer commented 5 years ago

the module in line [Transaction ] line 60 is vulnerable to SQL injection . the vulnerable end point is latr_/?orderBy=transId' order by input is being sent with out escape the query , allows attackers to execute malicious SQL Payloads .

amirsanni commented 5 years ago

Thanks for bringing this to my notice.

  1. Were you able to successfully inject SQL via that?
  2. I believe CodeIgniter's get() method will prevent the attack.
lawrenceamer commented 5 years ago

you passed $orderby input into transaction module as order by query with out escaping , could be fixed by binding , but i think will be better to re write safe one

amirsanni commented 5 years ago

The TRUE value passed to the second parameter of $this->input->get() is for XSS filtering. See image below from the documentation:

image

lawrenceamer commented 5 years ago

, i think will be better to use binding to avoid SQL injection on selected module https://www.codeigniter.com/userguide2/database/queries.html , or wait my patch for that , you can close this ticket as Un resolved , thanks

amirsanni commented 5 years ago

That's exactly what I used in the model. Check the getAll() here.

lawrenceamer commented 5 years ago

try this on your browser locally with Mysql Databases , http://demo.com/transactions/latr_/?orderBy=transId%27&orderFormat=DESC&limit=10