WHMCSPeter
commented
3 years ago Hi @reza-latifi,
Thank you for opening your issue. We appreciate everyone who reaches out with legitimate concerns such as yours.
For any potential "Security Vulnerability" reports, we do ask that you kindly report these via our Bug Bounty Program in a responsible manner via https://bugcrowd.com/whmcs and not via a public forum such as this.
The masking within the module log is supposed to be provided as an extra hardening step and not for absolute security. Per our documentation, we do suggest immediately disabling and clearing the Module Debug Log when not in use for these reasons.
It's believed that if someone has managed to access your Module Log via WHMCS or via the database, you most likely have other security issues to address also.
It is never advised to leave the Module Log always recording. The tables storing this data can become very large, very quickly. There is no guarantee that sensitive information will be completely absent from low-level communication logging either. We provide this masking as a rudimentary means to prevent issues of the worst case scenario.
Once again, I'd like to thank you for taking the time to write this report. If you do feel this issue warrants further investigation, you are welcome to reach out to our Technical Support team or submit a report via BugCrowd / https://bugs.whmcs.com/.
The following method is proposed for logging module calls:
logModuleCall($module, $action, $requestString, $responseData, $processedData, $replaceVars);The last parameter of the method accepts an array of strings for replacement. Thus, one can use it to hide the strings they don't want to be shown in logs. Let's consider I want to log API calls in my module. The value of "password" is "secret1234" and I pass the values of the "password", "username", "email", and "accesskey" to the logger to be replaced.
The first and foremost problem is in the "Filtering by value" approach. If the value of "email" consists of "secret1234", that part of the email will be replaced with "*"s. Then one can do "Dictionary Attack" by giving various values as the email address to the API. My suggestion is to filter by key.
The second problem is that the "logModuleCall" changes the value of "password" to "**", so the length of the "password" is visible. My suggestion is to change all given strings to a fixed number of "*"s.
The following is the code I used to filter values prior to logging.
$sensetiveParams = ['username', 'password', 'identifier', 'secret', 'accesskey']; foreach($sensetiveParams as $param){ if(isset($params[$param])){ $params[$param] = '***'; } }