Peter O'Callaghan
Thoughts on Development, Magento and Security
  • email
  • twitter
  • Home
  • About
Select Page ...

Logs, Logs, Wherefore Art Thou Logs?

December 6, 2017 Magento

The recently released SUPEE-10415 patch introduced some changes relating to validation of the log file paths before writing to them (APPSEC-1913). There is one side affect of these changes that I’ve not seen anybody talking about so I thought I’d write this post.

The consequence I’m refering to is caused by this change, in Mage.php:

Change in Mage.php
diff
1
2
-        $file = empty($file) ? 'system.log' : $file;
+        $file = empty($file) ? 'system.log' : basename($file);

It is fairly common for 3rd party extensions to try to ‘namespace’ their log files by putting them in sub-directories. This is normally achieved by doing something along the lines of:

Change in Mage.php
diff
1
Mage::log('some log data', null, 'my_ext/general.log');

The introduction of basename in the log method results in the my_ext/ part being stripped off and thus the log entry some log data is written to var/log/general.log.

As an isolated change this is not a big issue, though it will likely cause some confusion as to why log files have seemingly ‘stopped logging’, when in reality they have just written to a different filepath than expected. There are, however, some things that need to be considered from a custom code / infrastructure standpoint:

  1. If you use generic filenames, your log data will be merged with the log data of other extensions.
  2. Custom functionality you have for displaying your log data in the admin panel*, will likely read the content from the wrong file, thus not show the desired information.
  3. Log rotate may be either not rotating the files any more or using different configuration settings to before.
  4. Clean-up crons / actions for removing old log data may no longer remove the correct file.
  5. Attempts to parse data from your log files will no longer use the right path.

* From my experience this kind of functionality is often vulnerable to ‘stored XSS’, please make sure you’re escaping any log data before outputting it to the page. Even if it’s currently impossible to manipulate the logged data, some future update may change that and you’re unlikely to remember to come back and fix this.

In the long term, to avoid these issues, it likely makes sense to namespace your filenames, rather than through sub-directories. This would make our example file my_ext_general.log.

As a final note to 3rd party developers; The patch also introduced limitations on the file extensions that can be used. So for example, if you’re logging JSON data for consumption by a log reader, you’ll want to use .log still rather than .json.

← APPSEC-1281 and Dangerous Symlinks?
  • Categories

    • Development Process
    • Magento
    • Sysadmin
    • Uncategorized
  • Tags

    .htacces Apache composer facepalm filesystem magento 2 mod_rewrite security
    • Archives

      • December 2017
      • June 2017
      • April 2017
      • February 2017
      • January 2017
      • December 2016
      • October 2016
      • September 2016
      • July 2016
      • February 2016
      • January 2016
      • April 2012
      • February 2012
      • August 2011
    • Categories

      • Development Process
      • Magento
      • Sysadmin
      • Uncategorized
    • “There are two types of people. Those who can extrapolate from incomplete data.”

    • Contact
    • Home
    Copyright © 2012 All Rights Reserved - peterocallaghan.co.uk