Suitecrm: Possible opportunity for SQL injection attack in file modules/Emails/EmailUIAjax.php

Created on 10 Jun 2017  路  2Comments  路  Source: salesagility/SuiteCRM

inside:
case "getTemplateAttachments":

line:
$where = "parent_id='{$_REQUEST['parent_id']}'";

All user inputs must be used after validation/sanitisation/escaping in SQL commands.
Refer to usage of DBManager::quote() function

Critical Fix Proposed Bug

Most helpful comment

You're absolutely right @muratyaman

  • I'm 100% sure this one bug isn't the only SQL injection security vulnerability hidden in plain sight in the open source code of this 14 yr old PHP app, developed at a time when securing CRM data against organized global hacker botnets was an afterthought, downplayed, and dismissed.
  • We must immediately encourage our dear, anonymous management team of @SalesAgility to connect the free automated code analysis tools to this repo immediately for the purposes of flagging all of these SQL Injection security holes. Scrutinizer-CI, PHP-CS, etc. See my previous 10 or so posts going back 6+ months.
  • We must immediately ask @salesagility to answer the question from many here, what else do you need from us to merge #3237 , Composer, Symfony PHP's brilliant standard 3rd Party Library Dependency Manager, which would do a great deal to improve the automatic management of external code libraries used by this PHP app by using software technology from today.
  • Quality Improvement and Information Security are the best reasons why it's so urgent to merge these PRs into master, develop, and hotfix: #3600 #3689 (Work In Progress - not ready yet - started by #3325 ) and #3291 (Security Anti-hacking).

All 2 comments

You're absolutely right @muratyaman

  • I'm 100% sure this one bug isn't the only SQL injection security vulnerability hidden in plain sight in the open source code of this 14 yr old PHP app, developed at a time when securing CRM data against organized global hacker botnets was an afterthought, downplayed, and dismissed.
  • We must immediately encourage our dear, anonymous management team of @SalesAgility to connect the free automated code analysis tools to this repo immediately for the purposes of flagging all of these SQL Injection security holes. Scrutinizer-CI, PHP-CS, etc. See my previous 10 or so posts going back 6+ months.
  • We must immediately ask @salesagility to answer the question from many here, what else do you need from us to merge #3237 , Composer, Symfony PHP's brilliant standard 3rd Party Library Dependency Manager, which would do a great deal to improve the automatic management of external code libraries used by this PHP app by using software technology from today.
  • Quality Improvement and Information Security are the best reasons why it's so urgent to merge these PRs into master, develop, and hotfix: #3600 #3689 (Work In Progress - not ready yet - started by #3325 ) and #3291 (Security Anti-hacking).

Thanks @muratyaman - Looks like that has been there since sugar days but that's no excuse. We have announced within the forums that we are currently addressing the absent of tools and automated testing in dedicated sprints in the 7.10 development cycle which will introduce and put into place the most suitable tools for the team and the community along with processes. Regards to raising security issues - we have updated the security process very recently so please see the wiki (the README will be updated upon release) for the email to send to going forward.

Just on a last note we are a relatively small product team and we appreciate all the help that the community provides, but if you and other members can understand that we are not a 20 man team and we can only do things when we are able to. So that being said when this version has reached a stable release then we can address the open PRs that focus on improving the code quality:that is our highest to do after this 7.9 release.

Was this page helpful?
0 / 5 - 0 ratings