search

ChyroBroadcast / ftp-adm-api

Ftp Admin Backend
GNU Affero General Public License v3.0
0 stars 0 forks source link

updateUser is not safe against SQL Injection #3

Open billy482 opened 8 years ago

billy482 commented 8 years ago

The main default is in the foreach loop, data received should not be concatenated into a string which will be prepared or executed after. Data should always be sanitized before executing any sql request.

php: sql injection

enzzc commented 8 years ago

Does commit a5de387b298ac1b40d39a97442297e9767c7a511 solves this issue? The request is built using ':'-prefixed parameters before execution. Is this enough to ensure safety?

billy482 commented 8 years ago

It's better than previous version. @enzzc, you can use a tool like SQLmap, GitHub repository to check against sql injection. In GitHub, when you do a commit, we have nice feature like issue #3 which create a link from commit message. Another nice feature, close issue from commit message.

billy482 commented 8 years ago

To avoid sql errors, it should be better to have an array which contains all fields of table user and make sql statement by iterating over this array and lookup into $fields to add into $to_be_updated. In one's hand, if $fields contains a pair of value ('foo' => 'bar'), the execution of query will failed. In other's hand, this check should be done in other place.