Full path disclosure due to Undefined index

[mrnfrancesco]

In admin panel, if you are not logged in, and generate a 404 Not Found error with something like <youdomain>/admin/index.php?route=common/login/XXX an Undefined index warning will appears leading to Full Path Disclosure issue.

[Milow]

I don't see how this can happen, and I can't reproduce.

Line 158 of admin/index.php:
$controller->addPreAction(new Action('common/login/check'));

Which is going run the check function on line 103 of admin/controller/common/login.php. It's going to fail the check at line 124 because you are not logged-in and the route is not ignored, so now we are sent to the login page. There is no way to hit the error/not_found.php file if you are not logged in.

Please advise me if I am wrong, but like I said, I cannot reproduce this issue.

[mrnfrancesco]

As you can see in the image above I see the warning coming from error/not_found.php.

I debugged opencart and I think the problem is I'm asking for a page that does not require login (route=common/login), so OC start searching for a method named XXX and hit the error/not_found.php.

Does it make sense?
(It is possible that you cannot reproduce this issue if you have a well configured server that does not print warnings/errors)

[Milow]

Good catch. Only seems to be reproducible with a route that is route=common/login/xxx or in the ignore array on line 118 of admin/controller/common/login.php

A possible solution that may be better would be to change the check inside of the check() function of login.php to something like:

if (isset($this->request->get['route'])) {
    $part = explode('/', $this->request->get['route']);

    $num_parts = count($part);

    if ($num_parts == 1) {
        $route .= $part[0];
    } else if ($num_parts > 1) {
        $route .= $part[0];

        for ($i = 1; $i < $num_parts; $i++) {
            $route .= '/' . $part[$i];
        }
    }
}

This allows the pages that need to be accessed in the ignore table to be accessed, while rejecting weird things such as route=common/login/somethinghere

[mrnfrancesco]

You're right, it is a better solution. I'll update my pull request in minutes.

[Milow]

Didn't think about the simple solution - nice.

[jamesallsup]

Perfect, thanks.