(Security) problems with new feed-me logging

[jeffreyzant]

Description

The newly implemented feed-me logging has several issues:

1. Sensitive Data Exposure: When making a request to the same Craft application, the logger adds an entry to the database with the application category. This entry includes the entire $_SERVER array, which may contain plain text environment variables with passwords. If this database table is exposed (e.g., through a backup), all credentials (database, encryption keys, ...) could be compromised.

2. Logging Behavior: The example provided in the release does not seem to prevent logs from being written to the database. Even though log files are written, adjusting the monologTargetConfig as stated in the README does not stop the feed-me logs from being recorded in the database.

3. Log Clearing Limitations: When adjusting the log target there is no way to clear the logs through the frontend as the clear method only does a truncate of the database table. This limitation makes it difficult to manage and remove logs from the frontend.

4. Lack of Log Rotation: The current implementation lacks log rotation, leading to uncontrolled log growth. Some of our applications have already accumulated over 2GB of logs.

Overall, this implementation is flawed and constitutes a breaking change that should not have been introduced by default in a minor version update.

Additional info

• Craft version: 4.11.2
• PHP version: 8.3.*
• Database driver & version: MySQL
• Plugins & versions: Feed-me: 5.6.1

[jeffreyzant]

For now we've adjusted our components config to the following below and disabled the logging in the feed-me.php config file (or you can disable the logging tab, which does the same). Some notes:

• The logs tab will not show the logs written to the log files as this only loads records from the database.
• As we use the FileTarget default Yii log rotation takes place.
• Important is to also set 'logVars' => [] in the log config. Otherwise the $_SERVER environment vars are still leaked to the log file. (Craft usually sanitizes this, but not when using \yii\log\FileTarget directly.

feed-me.php

return [
    '*' => [
        'logging' => false,
    ],
];

app.php

'log' => [
    'monologTargetConfig' => [
        'except' => ['feed-me'],
    ],

    'targets' => [
        [
            'class' => \yii\log\FileTarget::class,
            'logFile' => '@storage/logs/feed-me.log',
            'categories' => ['feed-me'],
            'logVars' => [],
        ],
    ],
]

[jamesmacwhite]

See #1487 for similar areas around excessive amount of logs causing issues.

[timkelty]

@jeffreyzant Thanks for the thorough report and workaround. I think your assessment is spot-on and something we need to address. In truth, the old logging behavior wasn't working for many people (being file-based it didn't work on load-balanced envs) that I think we rushed an oversimplified fix out to something that shouldn't have been a minor bump, or should have been opt-in.

We'll work on a remedy ASAP – stay tuned.

[timkelty]

As a start:

• I've released 5.6.2/6.2.2, which will no longer log any vars (as it didn't before the switch to database logging).
• I've also included a warning in the changelog for the versions that changed the logging behavior to the database.
• I've updated the language and example in README
  • Make it clear you need to disable logging and add a log target if you wish to log to file instead of database.
  • Change the example to use \craft\log\MonologTarget, which will give you log file rotation (similar to previous versions of Feed Me)

Solutions for bloated logs tables issues are being discussed here: #1487

[timkelty]

@jeffreyzant Everything mentioned here should be resolved in the latest releases:

• Sensitive Data Exposure - logVars are no longer logged
• Logging Behavior - Example and language in README has been updated
• Log Clearing Limitations - By design, you'll only be able to clear the db logs – however, see Log Rotation note, which will prevent uncontrolled accumulation of logs
• Lack of Log Rotation - Example now shows implementation that uses the same log rotation that Craft uses by default

A follow up feature is #1494, which will prevent the db table from bloating if you leave db logging enabled.

Thanks again for the report, and sorry for the trouble it caused.

[jeffreyzant]

Thanks for the quick action you took!

[arentsen]

I am using the code from the README file, but getting the following error:

Setting read-only property: craft\web\Application::log

205206207208209210211212213214215216217218219220221222223        $this->ensureBehaviors();
        foreach ($this->_behaviors as $behavior) {
            if ($behavior->canSetProperty($name)) {
                $behavior->$name = $value;
                return;
            }
        }
 
        if (method_exists($this, 'get' . $name)) {
            throw new InvalidCallException('Setting read-only property: ' . get_class($this) . '::' . $name);
        }
 
        throw new UnknownPropertyException('Setting unknown property: ' . get_class($this) . '::' . $name);
    }
 
    /**
     * Checks if a property is set, i.e. defined and not null.
     *
     * This method will check in the following order and act accordingly:

Any help would be appreciated.

[timkelty]

@arentsen that error makes it sound like you are missing the components key your config/app.php file.

[arentsen]

@timkelty great, I tried to find out what that means and how to fix it without being successful. Further instructions by anyone would be very helpful.

[timkelty]

@arentsen can you share the entirety of your config/app.php file?

[arentsen]

return [
    'components' => [
        'queue' => [
            'ttr' => 1200,
        ],
    ],
    'modules' => [
        'contact' => \modules\contact_form\Module::class,
    ],

    
];

[timkelty]

I am using the code from the README file, but getting the following error:

@arentsen And what are you looking to achieve exactly? If you are looking to disable database logging and log instead to a file (as discussed in this thread), you haven't included any of the example code from the README as you mentioned.

Doing so would look something like this:

config/feed-me.php

<?php
return [
    // disable default logging to database
    'logging' => false,
];

config/app.php

<?php
return [
    'components' => [
        'queue' => [
            'ttr' => 1200,
        ],
        'log' => [
            'monologTargetConfig' => [
                // optionally, omit from Craft's default logs
                'except' => ['feed-me'],
            ],
            
            // add your own log target to write logs to file
            'targets' => [
                [
                    // log to file or STDOUT/STDERR if CRAFT_STREAM_LOG=1 is set
                    'class' => \craft\log\MonologTarget::class,
                    'name' => 'feed-me',
                    'categories' => ['feed-me'],
                    
                    // Don't log request and env vars
                    'logContext' => false,
                    
                    // Minimum level to log
                    'level' => \Psr\Log\LogLevel::INFO,
                ],
            ],
        ],
    ],
    'modules' => [
        'contact' => \modules\contact_form\Module::class,
    ],
];

[arentsen]

Great, now it works, thanks!! I had added that part in the app.php file but commented it out since it gave an error, so that's why I did not copy it in, sorry.