* [MOD] Improved public links security by encrypting the whole account data

2026-03-06 16:36:59 +01:00 · 2017-02-22 21:36:42 +01:00
parent 508fd7c2b2
commit 49ba3d8bfb
8 changed files with 183 additions and 70 deletions
--- a/inc/SP/Account/Account.class.php
+++ b/inc/SP/Account/Account.class.php
@@ -365,18 +365,19 @@ class Account extends AccountBase implements AccountInterface
    /**
     * Incrementa el contador de visitas de una cuenta en la BBDD
     *
+     * @param int $id
     * @return bool
     * @throws \SP\Core\Exceptions\QueryException
     * @throws \SP\Core\Exceptions\ConstraintException
     */
-    public function incrementViewCounter()
+    public function incrementViewCounter($id = null)
    {
        $query = /** @lang SQL */
            'UPDATE accounts SET account_countView = (account_countView + 1) WHERE account_id = ? LIMIT 1';

        $Data = new QueryData();
        $Data->setQuery($query);
-        $Data->addParam($this->accountData->getAccountId());
+        $Data->addParam($id ?: $this->accountData->getAccountId());

        return DB::getQuery($Data);
    }
@@ -384,18 +385,19 @@ class Account extends AccountBase implements AccountInterface
    /**
     * Incrementa el contador de vista de clave de una cuenta en la BBDD
     *
+     * @param null $id
     * @return bool
     * @throws \SP\Core\Exceptions\QueryException
     * @throws \SP\Core\Exceptions\ConstraintException
     */
-    public function incrementDecryptCounter()
+    public function incrementDecryptCounter($id = null)
    {
        $query = /** @lang SQL */
            'UPDATE accounts SET account_countDecrypt = (account_countDecrypt + 1) WHERE account_id = ? LIMIT 1';

        $Data = new QueryData();
        $Data->setQuery($query);
-        $Data->addParam($this->accountData->getAccountId());
+        $Data->addParam($id ?: $this->accountData->getAccountId());

        return DB::getQuery($Data);
    }
@@ -473,4 +475,44 @@ class Account extends AccountBase implements AccountInterface

        return DB::getResults($Data);
    }
+
+    /**
+     * Obtener los datos de una cuenta.
+     * Esta funcion realiza la consulta a la BBDD y guarda los datos en las variables de la clase.
+     *
+     * @return AccountExtData
+     * @throws \SP\Core\Exceptions\SPException
+     */
+    public function getDataForLink()
+    {
+        $query = /** @lang SQL */
+            'SELECT account_name,'
+            . 'account_login,'
+            . 'account_pass,'
+            . 'account_key,'
+            . 'account_url,'
+            . 'account_notes,'
+            . 'category_name,'
+            . 'customer_name '
+            . 'FROM accounts '
+            . 'LEFT JOIN customers ON account_customerId = customer_id '
+            . 'LEFT JOIN categories ON account_categoryId = category_id '
+            . 'WHERE account_id = ? LIMIT 1';
+
+        $Data = new QueryData();
+        $Data->setQuery($query);
+        $Data->setMapClass($this->accountData);
+        $Data->addParam($this->accountData->getAccountId());
+
+        /** @var AccountExtData|array $queryRes */
+        $queryRes = DB::getResults($Data);
+
+        if ($queryRes === false) {
+            throw new SPException(SPException::SP_CRITICAL, __('No se pudieron obtener los datos de la cuenta', false));
+        } elseif (is_array($queryRes) && count($queryRes) === 0) {
+            throw new SPException(SPException::SP_CRITICAL, __('La cuenta no existe', false));
+        }
+
+        return $this->accountData;
+    }
 }
--- a/inc/SP/Controller/AccountController.class.php
+++ b/inc/SP/Controller/AccountController.class.php
@@ -41,6 +41,7 @@ use SP\Core\Init;
 use SP\Core\Session;
 use SP\Core\SessionUtil;
 use SP\Core\Template;
+use SP\DataModel\AccountData;
 use SP\DataModel\AccountExtData;
 use SP\DataModel\CustomFieldData;
 use SP\DataModel\PublicLinkData;
@@ -122,22 +123,12 @@ class AccountController extends ControllerBase implements ActionsInterface
     * Obtener la vista de detalles de cuenta para enlaces públicos
     *
     * @param PublicLinkData $PublicLinkData
-     * @return bool
-     * @throws \SP\Core\Exceptions\SPException
-     * @throws \SP\Core\Exceptions\FileNotFoundException
-     * @throws \SP\Core\Exceptions\QueryException
-     * @throws \SP\Core\Exceptions\ConstraintException
     *
     */
    public function getAccountFromLink(PublicLinkData $PublicLinkData)
    {
        $this->setAction(self::ACTION_ACC_VIEW);

-        // Obtener los datos de la cuenta antes y comprobar el acceso
-        if (!$this->setAccountData()) {
-            return false;
-        }
-
        $this->view->addTemplate('account-link');
        $this->view->assign('title',
            [
@@ -146,50 +137,27 @@ class AccountController extends ControllerBase implements ActionsInterface
                'icon' => $this->icons->getIconView()->getIcon()
            ]
        );
-        $this->Account->incrementViewCounter();
-        $this->Account->incrementDecryptCounter();
-        $AccountPassData = $this->Account->getAccountPassData();

-        // Obtener la llave de la clave maestra
-        $securedKey = Crypt::unlockSecuredKey($PublicLinkData->getPassIV(), Config::getConfig()->getPasswordSalt() . $PublicLinkData->getLinkHash());
+        try {
+            $Account = new Account();
+            $Account->incrementViewCounter($PublicLinkData->getItemId());
+            $Account->incrementDecryptCounter($PublicLinkData->getItemId());

-        // Desencriptar la clave de la cuenta
-        $accountSecuredKey = Crypt::unlockSecuredKey($AccountPassData->getAccountKey(), Crypt::decrypt($PublicLinkData->getPass(), $securedKey));
-        $accountPass = Crypt::decrypt($AccountPassData->getAccountPass(), $accountSecuredKey);
+            $key = Config::getConfig()->getPasswordSalt() . $PublicLinkData->getLinkHash();
+            $securedKey = Crypt::unlockSecuredKey($PublicLinkData->getPassIV(), $key);

-        $this->view->assign('useImage', Config::getConfig()->isPublinksImageEnabled() || Config::getConfig()->isAccountPassToImage());
+            /** @var AccountExtData $AccountData */
+            $AccountData = unserialize(Crypt::decrypt($PublicLinkData->getData(), $securedKey, $key));

-        if ($this->view->useImage) {
-            $accountPass = ImageUtil::convertText($accountPass);
+            $this->view->assign('useImage', Config::getConfig()->isPublinksImageEnabled() || Config::getConfig()->isAccountPassToImage());
+
+            $accountPass = $this->view->useImage ? ImageUtil::convertText($AccountData->getAccountPass()) : $AccountData->getAccountPass();
+
+            $this->view->assign('accountPass', $accountPass);
+            $this->view->assign('accountData', $AccountData);
+        } catch (\Exception $e) {
+            $this->showError(self::ERR_EXCEPTION);
        }
-
-        $this->view->assign('accountPass', $accountPass);
-    }
-
-    /**
-     * Establecer las variables que contienen la información de la cuenta.
-     *
-     * @throws \SP\Core\Exceptions\SPException
-     */
-    private function setAccountData()
-    {
-        $Account = new Account(new AccountExtData($this->getId()));
-        $this->Account = $Account;
-        $this->AccountData = $Account->getData();
-
-        $this->view->assign('accountId', $this->getId());
-        $this->view->assign('accountData', $this->AccountData);
-        $this->view->assign('gotData', $this->isGotData());
-
-        return true;
-    }
-
-    /**
-     * @return int
-     */
-    private function getId()
-    {
-        return $this->id;
    }

    /**
@@ -317,8 +285,9 @@ class AccountController extends ControllerBase implements ActionsInterface

            $PublicLinkData = PublicLink::getItem()->getHashForItem($this->getId());

-            $publicLinkUrl = (Checks::publicLinksIsEnabled() && $PublicLinkData ? Init::$WEBURI . '/index.php?h=' . $PublicLinkData->getPublicLinkHash() . '&a=link' : '');
+            $publicLinkUrl = (Checks::publicLinksIsEnabled() && $PublicLinkData ? Init::$WEBURI . '/index.php?h=' . $PublicLinkData->getPublicLinkHash() . '&a=link' : null);
            $this->view->assign('publicLinkUrl', $publicLinkUrl);
+            $this->view->assign('publicLinkId', $PublicLinkData->getPublicLinkId());

            $this->view->assign('accountPassDate', date('Y-m-d H:i:s', $this->AccountData->getAccountPassDate()));
            $this->view->assign('accountPassDateChange', date('Y-m-d', $this->AccountData->getAccountPassDateChange() ?: 0));
@@ -391,6 +360,32 @@ class AccountController extends ControllerBase implements ActionsInterface
        $this->setCommonData();
    }

+    /**
+     * Establecer las variables que contienen la información de la cuenta.
+     *
+     * @throws \SP\Core\Exceptions\SPException
+     */
+    private function setAccountData()
+    {
+        $Account = new Account(new AccountExtData($this->getId()));
+        $this->Account = $Account;
+        $this->AccountData = $Account->getData();
+
+        $this->view->assign('accountId', $this->getId());
+        $this->view->assign('accountData', $this->AccountData);
+        $this->view->assign('gotData', $this->isGotData());
+
+        return true;
+    }
+
+    /**
+     * @return int
+     */
+    private function getId()
+    {
+        return $this->id;
+    }
+
    /**
     * Obtener los datos para mostrar el interface para editar cuenta
     *
--- a/inc/SP/Controller/ItemActionController.class.php
+++ b/inc/SP/Controller/ItemActionController.class.php
@@ -692,6 +692,9 @@ class ItemActionController implements ItemControllerInterface
     * @throws \SP\Core\Exceptions\SPException
     * @throws \SP\Core\Exceptions\InvalidClassException
     * @throws \phpmailer\phpmailerException
+     * @throws \Defuse\Crypto\Exception\BadFormatException
+     * @throws \Defuse\Crypto\Exception\CryptoException
+     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
     */
    protected function publicLinkAction()
    {
--- a/inc/SP/DataModel/PublicLinkData.class.php
+++ b/inc/SP/DataModel/PublicLinkData.class.php
@@ -81,6 +81,10 @@ class PublicLinkData extends PublicLinkBaseData
     * @var array
     */
    protected $useInfo = [];
+    /**
+     * @var string
+     */
+    protected $data;

    /**
     * @return int
@@ -289,4 +293,20 @@ class PublicLinkData extends PublicLinkBaseData
    {
        $this->useInfo[] = $useInfo;
    }
+
+    /**
+     * @return string
+     */
+    public function getData()
+    {
+        return $this->data;
+    }
+
+    /**
+     * @param string $data
+     */
+    public function setData($data)
+    {
+        $this->data = $data;
+    }
 }
--- a/inc/SP/Mgmt/PublicLinks/PublicLink.class.php
+++ b/inc/SP/Mgmt/PublicLinks/PublicLink.class.php
@@ -110,6 +110,11 @@ class PublicLink extends PublicLinkBase implements ItemInterface

    /**
     * @return $this
+     * @throws \SP\Core\Exceptions\QueryException
+     * @throws \SP\Core\Exceptions\ConstraintException
+     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
+     * @throws \Defuse\Crypto\Exception\CryptoException
+     * @throws \Defuse\Crypto\Exception\BadFormatException
     * @throws SPException
     */
    public function add()
@@ -123,7 +128,7 @@ class PublicLink extends PublicLinkBase implements ItemInterface
        $this->itemData->setMaxCountViews(Config::getConfig()->getPublinksMaxViews());
        $this->calcDateExpire();
        $this->createLinkHash();
-        $this->createLinkPass();
+        $this->setLinkData();

        $query = /** @lang SQL */
            'INSERT INTO publicLinks
@@ -186,6 +191,9 @@ class PublicLink extends PublicLinkBase implements ItemInterface

    /**
     * @return $this
+     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
+     * @throws \Defuse\Crypto\Exception\CryptoException
+     * @throws \Defuse\Crypto\Exception\BadFormatException
     * @throws SPException
     */
    public function refresh()
@@ -194,7 +202,7 @@ class PublicLink extends PublicLinkBase implements ItemInterface

        $this->calcDateExpire();
        $this->createLinkHash(true);
-        $this->createLinkPass();
+        $this->setLinkData();

        $query = /** @lang SQL */
            'UPDATE publicLinks
@@ -359,7 +367,7 @@ class PublicLink extends PublicLinkBase implements ItemInterface
    public function getHashForItem($itemId)
    {
        $query = /** @lang SQL */
-            'SELECT publicLink_hash FROM publicLinks WHERE publicLink_itemId = ? LIMIT 1';
+            'SELECT publicLink_id, publicLink_hash FROM publicLinks WHERE publicLink_itemId = ? LIMIT 1';

        $Data = new QueryData();
        $Data->setMapClassName($this->getDataModel());
--- a/inc/SP/Mgmt/PublicLinks/PublicLinkBase.class.php
+++ b/inc/SP/Mgmt/PublicLinks/PublicLinkBase.class.php
@@ -26,10 +26,12 @@ namespace SP\Mgmt\PublicLinks;

 defined('APP_ROOT') || die();

+use SP\Account\Account;
 use SP\Config\Config;
 use SP\Core\Crypt\Crypt;
 use SP\Core\Crypt\Session as CryptSession;
 use SP\Core\Exceptions\SPException;
+use SP\DataModel\AccountExtData;
 use SP\DataModel\PublicLinkData;
 use SP\Mgmt\ItemBase;
 use SP\DataModel\PublicLinkBaseData;
@@ -84,6 +86,33 @@ abstract class PublicLinkBase extends ItemBase
        $this->itemData->setPassIV($securedKey);
    }

+    /**
+     * Obtener los datos de una cuenta y encriptarlos para el enlace
+     *
+     * @throws \Defuse\Crypto\Exception\CryptoException
+     * @throws \SP\Core\Exceptions\SPException
+     * @throws \Defuse\Crypto\Exception\BadFormatException
+     * @throws \Defuse\Crypto\Exception\EnvironmentIsBrokenException
+     */
+    protected final function setLinkData()
+    {
+        // Obtener los datos de la cuenta
+        $Account = new Account(new AccountExtData($this->itemData->getItemId()));
+        $AccountData = $Account->getDataForLink();
+
+        $key = CryptSession::getSessionKey();
+        $securedKey = Crypt::unlockSecuredKey($AccountData->getAccountKey(), $key);
+        $AccountData->setAccountPass(Crypt::decrypt($AccountData->getAccountPass(), $securedKey, $key));
+        $AccountData->setAccountKey(null);
+
+        // Encriptar los datos de la cuenta
+        $linkKey = Config::getConfig()->getPasswordSalt() . $this->createLinkHash();
+        $linkSecuredKey = Crypt::makeSecuredKey($linkKey);
+
+        $this->itemData->setData(Crypt::encrypt(serialize($AccountData), $linkSecuredKey, $linkKey));
+        $this->itemData->setPassIV($linkSecuredKey);
+    }
+
    /**
     * Generar el hash para el enlace
     *
--- a/inc/themes/material-blue/views/account/actions.inc
+++ b/inc/themes/material-blue/views/account/actions.inc
@@ -43,18 +43,34 @@ use SP\Util\Checks;
        <?php endif; ?>

        <?php if ($AccountAcl->isShowLink() && $AccountAcl->isShowViewPass() && $accountData->getAccountParentId() === 0 && $accountIsHistory !== 1): ?>
-            <li>
-                <button id="btnLink" type="button"
-                        class="btn-action mdl-button mdl-js-button mdl-button--fab mdl-button--mini-fab mdl-button--colored <?php echo $icons->getIconPublicLink()->getClassButton(); ?>"
-                        data-action-id="<?php echo \SP\Core\ActionsInterface::ACTION_MGM_PUBLICLINKS_NEW; ?>"
-                        data-nextaction-id="<?php echo \SP\Core\ActionsInterface::ACTION_ACC_VIEW; ?>"
-                        data-item-id="<?php echo $accountId; ?>"
-                        data-sk="<?php echo $sk; ?>"
-                        data-onclick="link/save"
-                        title="<?php echo __('Crear Enlace Público'); ?>">
-                    <i class="material-icons"><?php echo $icons->getIconPublicLink()->getIcon(); ?></i>
-                </button>
-            </li>
+            <?php if ($publicLinkUrl === null): ?>
+                <li>
+                    <button id="btnLink" type="button"
+                            class="btn-action mdl-button mdl-js-button mdl-button--fab mdl-button--mini-fab mdl-button--colored <?php echo $icons->getIconPublicLink()->getClassButton(); ?>"
+                            data-action-id="<?php echo \SP\Core\ActionsInterface::ACTION_MGM_PUBLICLINKS_NEW; ?>"
+                            data-nextaction-id="<?php echo \SP\Core\ActionsInterface::ACTION_ACC_VIEW; ?>"
+                            data-item-id="<?php echo $accountId; ?>"
+                            data-sk="<?php echo $sk; ?>"
+                            data-onclick="link/save"
+                            title="<?php echo __('Crear Enlace Público'); ?>">
+                        <i class="material-icons"><?php echo $icons->getIconPublicLink()->getIcon(); ?></i>
+                    </button>
+                </li>
+            <?php else: ?>
+                <li>
+                    <button id="btnLink" type="button"
+                            class="btn-action mdl-button mdl-js-button mdl-button--fab mdl-button--mini-fab mdl-button--colored <?php echo $icons->getIconPublicLink()->getClassButton(); ?>"
+                            data-action-id="<?php echo \SP\Core\ActionsInterface::ACTION_MGM_PUBLICLINKS_REFRESH; ?>"
+                            data-nextaction-id="<?php echo \SP\Core\ActionsInterface::ACTION_ACC_VIEW; ?>"
+                            data-item-id="<?php echo $publicLinkId; ?>"
+                            data-activetab="<?php echo $accountId; ?>"
+                            data-sk="<?php echo $sk; ?>"
+                            data-onclick="link/refresh"
+                            title="<?php echo __('Actualizar Enlace'); ?>">
+                        <i class="material-icons"><?php echo $icons->getIconPublicLink()->getIcon(); ?></i>
+                    </button>
+                </li>
+            <?php endif; ?>
        <?php endif; ?>

        <?php if ($AccountAcl->isShowViewPass()): ?>
--- a/js/app-actions.min.js
+++ b/js/app-actions.min.js
@@ -1,5 +1,5 @@
 var $jscomp={scope:{},findInternal:function(c,d,k){c instanceof String&&(c=String(c));for(var f=c.length,g=0;g<f;g++){var l=c[g];if(d.call(k,l,g,c))return{i:g,v:l}}return{i:-1,v:void 0}}};$jscomp.defineProperty="function"==typeof Object.defineProperties?Object.defineProperty:function(c,d,k){if(k.get||k.set)throw new TypeError("ES3 does not support getters and setters.");c!=Array.prototype&&c!=Object.prototype&&(c[d]=k.value)};
-$jscomp.getGlobal=function(c){return"undefined"!=typeof window&&window===c?c:"undefined"!=typeof global&&null!=global?global:c};$jscomp.global=$jscomp.getGlobal(this);$jscomp.polyfill=function(c,d,k,f){if(d){k=$jscomp.global;c=c.split(".");for(f=0;f<c.length-1;f++){var g=c[f];g in k||(k[g]={});k=k[g]}c=c[c.length-1];f=k[c];d=d(f);d!=f&&null!=d&&$jscomp.defineProperty(k,c,{configurable:!0,writable:!0,value:d})}};
+$jscomp.getGlobal=function(c){return"undefined"!=typeof window&&window===c?c:"undefined"!=typeof global?global:c};$jscomp.global=$jscomp.getGlobal(this);$jscomp.polyfill=function(c,d,k,f){if(d){k=$jscomp.global;c=c.split(".");for(f=0;f<c.length-1;f++){var g=c[f];g in k||(k[g]={});k=k[g]}c=c[c.length-1];f=k[c];d=d(f);d!=f&&null!=d&&$jscomp.defineProperty(k,c,{configurable:!0,writable:!0,value:d})}};
 $jscomp.polyfill("Array.prototype.find",function(c){return c?c:function(c,k){return $jscomp.findInternal(this,c,k).v}},"es6-impl","es3");
 sysPass.Actions=function(c){var d=c.log,k=0,f={doAction:"/ajax/ajax_getContent.php",updateItems:"/ajax/ajax_getItems.php",user:{savePreferences:"/ajax/ajax_userPrefsSave.php",password:"/ajax/ajax_usrpass.php",passreset:"/ajax/ajax_passReset.php"},main:{login:"/ajax/ajax_doLogin.php",install:"/ajax/ajax_install.php",upgrade:"/ajax/ajax_upgrade.php",getUpdates:"/ajax/ajax_checkUpds.php"},checks:"/ajax/ajax_checkConnection.php",config:{save:"/ajax/ajax_configSave.php","export":"/ajax/ajax_configSave.php",
 "import":"/ajax/ajax_configSave.php"},file:"/ajax/ajax_filesMgmt.php",link:"/ajax/ajax_itemSave.php",plugin:"/ajax/ajax_itemSave.php",account:{save:"/ajax/ajax_itemSave.php",saveFavorite:"/ajax/ajax_itemSave.php",request:"/ajax/ajax_itemSave.php",getFiles:"/ajax/ajax_accGetFiles.php",search:"/ajax/ajax_accSearch.php"},appMgmt:{show:"/ajax/ajax_itemShow.php",save:"/ajax/ajax_itemSave.php",search:"/ajax/ajax_itemSearch.php"},eventlog:"/ajax/ajax_eventlog.php",wiki:{show:"/ajax/ajax_wiki.php"},notice:{show:"/ajax/ajax_noticeShow.php",