Skip to content

Fix for remote code execution #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
slawomir-pryczek wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix for remote code execution #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a65661d
* Fix for remote code execution
slawomir-pryczek Oct 5, 2017
3ddde54
Update README.md
slawomir-pryczek Oct 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Library/Bootstrap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
header('Cache-Control: no-cache, must-revalidate');

# Constants declaration
define('CURRENT_VERSION', '1.3.0');
define('CURRENT_VERSION', '1.3.1');

# PHP < 5.3 Compatibility
if (defined('ENT_IGNORE') === false) {
Expand Down
13 changes: 10 additions & 3 deletions Library/Data/Analysis.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,16 @@ public static function diff($array, $stats)

# Diff for each key
foreach ($stats as $key => $value) {
if (isset($array[$key])) {
$stats[$key] = $value - $array[$key];
}

if (!isset($array[$key]))
Copy link
Contributor

@fgm fgm Oct 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra spaces

continue;

# Make sure we're dealing with a real number
$v = $array[$key];
if (!is_float($v) && !is_int($v))
Copy link
Contributor

@fgm fgm Oct 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better add braces here (PSR-2 § 5)

continue;

$stats[$key] = $value - $v;
}

return $stats;
Expand Down
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# PHPMemcachedAdmin #

PHPMemcachedAdmin contains critical security bug (up to, and including v 1.3.0) which allows for remote code execution. This repository is aimed to increase PMA security by fixing this bug and (optionally) adding login option.

### Graphic stand-alone administration for memcached to monitor and debug purpose ###

This program allows to see in **real-time** (top-like) or from the start of the server, **stats for get, set, delete, increment, decrement, evictions, reclaimed, cas command**, as well as **server stats** (network, items, server version) with googlecharts and **server internal configuration**
Expand Down Expand Up @@ -45,4 +47,4 @@ Unzip/Untar & Give files permissions

You have to give **Read & Execute right to all files**, and **Read, Write & Execute to configuration files and temporary directory**.

More information in https://blog.elijaa.org/phpmemcachedadmin-installation-guide/
More information in https://blog.elijaa.org/phpmemcachedadmin-installation-guide/
20 changes: 20 additions & 0 deletions View/Stats/Stats.phtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,26 @@ if((isset($_REQUEST['server'])) && ($_ini->server($_REQUEST['server'])))
<span class="left setting help" title="Internal name : evicted_unfetched&#013;Items evicted from LRU that were never touched by get/incr/append/etc">Evicted unfetched</span>
<?php echo (isset($stats['evicted_unfetched'])) ? Library_Data_Analysis::hitResize($stats['evicted_unfetched']) : 'N/A on ' . $stats['version']; ?>
</div>

Copy link
Contributor

@fgm fgm Oct 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While useful, this block seems unrelated with the security issue. Removing it would make the PR smaller, hence easier to review and merge.

<? if (isset($stats['*reclaimed_fast'])) { // Additional attributes for custom memcached build ?>
<div class="line" style="background: #DDFFDD">
<span class="left setting help" title="Internal name : *reclaimed_fast&#013;Reclaimed fast from LRU, items that were reclaimed by memory scan">Reclaimed fast</span>
<?php echo (isset($stats['*reclaimed_fast'])) ? Library_Data_Analysis::hitResize($stats['*reclaimed_fast']) : 'N/A on ' . $stats['version']; ?>
</div>
<div class="line" style="background: #CCFFCC">
<span class="left setting help" title="Internal name : *reclaimed_fast_bytes&#013;Reclaimed fast from LRU, in bytes">Reclaimed fast bytes</span>
<?php echo (isset($stats['*reclaimed_fast_bytes'])) ? Library_Data_Analysis::hitResize($stats['*reclaimed_fast_bytes']) : 'N/A on ' . $stats['version']; ?>
</div>
<div class="line" style="background: #DDFFDD">
<span class="left setting help" title="Internal name : *reclaim_item_passes&#013;Reclaimed item passes, number of times memory was scanned for old items">Reclaimed item passes</span>
<?php echo (isset($stats['*reclaim_item_passes'])) ? Library_Data_Analysis::hitResize($stats['*reclaim_item_passes']) : 'N/A on ' . $stats['version']; ?>
</div>
<div class="line" style="background: #CCFFCC">
<span class="left setting help" title="Internal name : *reclaim_item_found&#013;Number of old items found using memory scan">Reclaim item found</span>
<?php echo (isset($stats['*reclaim_item_found'])) ? Library_Data_Analysis::hitResize($stats['*reclaim_item_found']) : 'N/A on ' . $stats['version']; ?>
</div>
<? } ?>

</div>

<?php
Expand Down
12 changes: 11 additions & 1 deletion stats.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,12 @@
# Cleaning temporary directory
$files = glob($_ini->get('file_path') . '*', GLOB_NOSORT);
foreach ($files as $path) {
# Getting file last modification time

# Only delete files which were created by us
if (strpos($path, ".mcatmp.txt") === false || strpos($path, "live_stats") === false)
continue;

# Getting file last modification time
$stats = @stat($path);

# Deleting file older than 24 hours
Expand All @@ -69,6 +74,11 @@
$live_stats_id = $_COOKIE['live_stats_id' . $hash];
}

# Prefix the file to not allow setting custom extension via cookie
# https://rstforums.com/forum/topic/85493-phpmemcachedadmin-122-remote-code-execution/
$live_stats_id = str_replace(chr(0), "", $live_stats_id);
$live_stats_id = "{$live_stats_id}.mcatmp.txt";

# Live stats dump file
$file_path = rtrim($_ini->get('file_path'), '/') . DIRECTORY_SEPARATOR . 'live_stats.' . $live_stats_id;

Expand Down