Hi Brian!
On 2016-12-16 03:44:51, Brian May wrote:
> I have patched a number of vulnerabilities in phpmyadmin in wheezy;
> there is a version available for testing at
> https://people.debian.org/~bam/debian/pool/main/p/phpmyadmin/
I haven't tested the package, but have reviewed your work, below.
> Not sure I entirely like the supplied fix from upstream for
> CVE-2016-4412 / PMASA-2016-57 - the included whitelist in
> PMA_isAllowedDomain has a number of domains included (because we use
> url.php to link to these domains, e.g. for external links to
> documentation). I see that the latest 4.6.5.1 version does the same
> thing.
>
> The included isAllowedDomain does not include some checks that are in
> later versions:
>
> $arr = parse_url($url);
> // We need host to be set
> if (! isset($arr['host']) || strlen($arr['host']) == 0) {
> return false;
> }
> // We do not want these to be present
> $blocked = array('user', 'pass', 'port');
> foreach ($blocked as $part) {
> if (isset($arr[$part]) && strlen($arr[$part]) != 0) {
> return false;
> }
> }
>
> It makes me nervous that these checks might be important.
I do think they are.
> I think I will need to revise CVE-2016-9861 / PMASA-2016-66 again,
> because it covers the same code. As CVE-2016-9861 / PMASA-2016-66 is
> considered a security issue, that implies the above checks are important
> for security. For reasons I don't entirely understand right now.
CVE-2016-9861 seems to be about a bypass of previous checks by using
false values in the "host" bit, for example. It consists of the
following patch:
https://github.com/phpmyadmin/phpmyadmin/commit/af7c589
This code was introduced in:
https://github.com/phpmyadmin/phpmyadmin/commit/e8c5cab3c117e68a0d837319e0e83bdfc50be1fb
To fix CVE-2016-6626 which was marked as "not-affected" in wheezy
because "vulnerable code not present"...
That said, I still think you should include them for the fix to be
complete. In the process, you could mark CVE-2016-6626 as affected as
well. :)
> The debdiff is below:
I have reviewed the diff summarily for logic errors and things seem fine
otherwise noted below.
> +--- a/url.php
> ++++ b/url.php
> ++ // JavaScript redirection is necessary. Because if header() is used
> ++ // then web browser sometimes does not change the HTTP_REFERER
> ++ // field and so with old URL as Referer, token also goes to
> ++ // external site.
I haven't reviewed the whole code - but this actually works? Doesn't
this assume the token isn't passed to the url.php file?
--
A people that elect corrupt politicians, imposters, thieves & traitors
are not victims... but accomplices.
- George Orwell
