userfrosting / UserFrosting

Modern PHP user login and management framework
https://www.userfrosting.com
Other
1.64k stars 366 forks source link

Possible Security issue, but could be just my environment. #1237

Closed Oirbsiu closed 11 months ago

Oirbsiu commented 11 months ago

UserFrosting version 4.6.4 PHP version 8.1.24 Webserver software Apache/2.4.57 (CentOS Stream) OpenSSL/3.0.7 Database version mysql 10.5.16-MariaDB Database name userfrosting Project directory /var/www/html/UserFrosting Site root url https://host Loaded sprinkles core account admin batman import

batman is my custom sprinkle - while trying to write some code to influence how the "download CSV" processes my data, i added some print_r() statements to help me debug the code.

   public function ParseCsv($array)
    {
        $collection = collect($array);

        $csv = Writer::createFromFileObject(new \SplTempFileObject());
        [$columnNames, $values] = Arr::divide($array[0]);
        // Flatten collection while simultaneously building the column names from the union of each element's keys */
        $collection->transform(function ($item, $key) use (&$columnNames) { 
            $item = Arr::dot($item);

            // if array key contains DN, parse DN string 
            if (array_key_exists('dn', $item)) {
                $item['dn'] = $this->ParseDN($item['dn']);
            }
            return $item;
        });

        $csv->insertOne($columnNames);

        // Insert the data as rows in the CSV document */
        $collection->each(function ($item, $key) use ($csv, $columnNames) {
            $eq = fn ($arr, $key) => $arr[$key];
            print_r($eq);
            $row = array_map(fn($o) => $eq($item, $o),$columnNames);
            $csv->insertOne($row);
        });
        return $csv;
    }

the response I got from the output of the $eq variable wasn't what I expeceted.

it pretty much showed me everything - including passwords saved in my .env Im kinda currious how $eq = fn ($arr, $key) => $arr[$key]; is slurping up everything???

is it a bug??

lcharette commented 11 months ago

Can you add more information how to replicate? Share a sample of the output (minus sensitive data)?

Oirbsiu commented 11 months ago

In my Sprinkle, Ive used some of the code taken from app/sprinkles/core/src/Sprunje/Sprunje.php - from public function getCsv()

and used it in my own code.

To replicate it, add this into app/sprinkles/core/src/Sprunje/Sprunje.php

            $eq = fn ($arr, $key) => $arr[$key];
            print_r($eq);

e.g.

        // Insert the data as rows in the CSV document
        $collection->each(function ($item) use ($csv, $columnNames) {
            $row = [];
            $eq = fn ($arr, $key) => $arr[$key];
            print_r($eq);
            foreach ($columnNames as $itemKey) {
                // Only add the value if it is set and not an array.  Laravel's array_dot sometimes creates empty child arrays :(
                // See https://github.com/laravel/framework/pull/13009
                if (isset($item[$itemKey]) && !is_array($item[$itemKey])) {
                    $row[] = $item[$itemKey];
                } else {
                    $row[] = '';
                }
            }

            $csv->insertOne($row);
        });

and then on any table, e.g. https://somesite.host/users click on the download CSV - the output is saved into the downloaded CSV

lcharette commented 11 months ago

In your example:

$eq = fn ($arr, $key) => $arr[$key];
print_r($eq);

print_r won't return the value of the array, but the closure itself. In this case, this is part of the closure, and this is were the ENV password comes from.

Capture d’écran, le 2023-12-10 à 20 41 45

So long story short, it's not a security issue, as it's not part of the $row, it's relevant to your code only.