[Phpmyadmin-devel] file_echo.php

[Dieter Adriaenssens]

Just a question about the code :

    $extension = $allowed[$_REQUEST['type']];
    $valid_match = '/^[^\n\r]*\.' . $extension . '$/';
    if (! preg_match($valid_match, $_REQUEST['filename'])) {
        if (! preg_match('/^[^\n\r]*$/', $_REQUEST['filename'])) {
            /* Add extension */
            $filename = 'dowload.' . $extension;
        } else {
            /* Filename is unsafe, discard it */
            $filename = $_REQUEST['filename'] . '.' . $extension;
        }

1) Shouldn't the two comments in the then/else be switched?
2) 'dowload', is this a typo?

Kind regards,

Dieter

2011/8/4 Tyron Madlener <tyronx at gmail.com>:
> Forgot again to write to pma-dev list
>
> On Thu, Aug 4, 2011 at 4:17 PM, Michal Čihař <michal at cihar.com> wrote:
>> Hi
>>
>> Dne Thu, 4 Aug 2011 15:47:59 +0300
>> Tyron Madlener <tyronx at gmail.com> napsal(a):
>>
>>> From what I can see, the token is being checked independent of what
>>> value PMA_MINUMUM_COMMON  is set to. Looking at the other parts of
>>> common.inc.php I also cannot see any security related functions not
>>> being executed if PMA_MINUMUM_COMMON is set. Also defining
>>> PMA_MINUMUM_COMMON I had added in the very first version of the file
>>> (when it was named chart_export.php), and from what I remember you
>>> overlooked that file there too.
>>>
>>> And I just tested that on the gsoc-tyron demo. It returns 'Invalid
>>> request' if no valid token is set.
>>>
>>> Apart from that, please elaborate, how can an attacker do harm to a
>>> user with my changes? And how is the user protected with
>>> PMA_MINUMUM_COMMON removed? Looking at common.inc.php, I fail to find
>>> any possible attack vector.
>>
>> Defining PMA_MINUMUM_COMMON skips MySQL authentication, so it might be
>> easier to exploit any possible issue, but you seem to be right that
>> token is checked.
>
> Do you have an example how the skipping of the MySQL authentication
> could be abused?
>
>>
>>> My added file echo for the monitor config forces the file name
>>> 'monitor.cfg', so even if the token is not checked, and an attacker is
>>> be able to trick a user to download a file no harm can be made, since
>>> .cfg Files are not executable or viewable by standard programs.
>>
>> This one looks pretty safe.
>>
>>> And the import-echo I added uses $_FILES which can only be set with an
>>> actual file upload. I wouldn't know how that could be exploited by an
>>> attacker.
>>
>> This one definitely allows XSS (though still protected by token, so
>> pretty harmless unless there is some other issue).
>
> The only way I could see how this can be used for an XSS attack
> (whether protected by token or not) is when the user himself uploads a
> malicious chart config file.
>
> We could counteract this by adding a notice "Do not use chart config
> files from untrusted sources!" at the chart config import dialog.
>
> And/Or I verify if the file really is pure json on the server side,
> before echoing it back. I guess doing 'echo
> json_encode(json_decode(file_get_contents(...)))' should do the trick.
>
> [Edit:] Actually verify the json wont help. One could still hide html
> code within a string.
>
>>Is the echo service for HTML really needed?
>>
>
> It is used for the chart config import part of the monitor. There you
> can upload a config file, which is within an iframe. Once submitted
> and echo'd back my js code reads the config and applies it to the
> monitor.
>
> It might be possible to read local files with a HTML5 feature, or
> maybe flash, but certainly will not work in all cases.
>
>>
>> --
>>        Michal Čihař | http://cihar.com | http://blog.cihar.com
>>
>
> ------------------------------------------------------------------------------
> BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
> The must-attend event for mobile developers. Connect with experts.
> Get tools for creating Super Apps. See the latest technologies.
> Sessions, hands-on labs, demos & much more. Register early & save!
> http://p.sf.net/sfu/rim-blackberry-1
> _______________________________________________
> Phpmyadmin-devel mailing list
> Phpmyadmin-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>



-- 
Groetjes,

Dieter Adriaenssens