smstw / windwalker-joomla-rad

Windwalker RAD framework for Joomla CMS
http://ventoviro.github.io/windwalker-rad-doc/
1 stars 2 forks source link

AssetHelper constructor 的 $path 型態有漏洞 #37

Closed LeoOnTheEarth closed 8 years ago

LeoOnTheEarth commented 9 years ago
/**
 * Constructor.
 *
 * @param string             $name  The instance name.
 * @param \SplPriorityQueue  $paths Paths to scan assets.
 */
public function __construct($name = 'windwalker', $paths = null)
{
    $this->name = $name;

    // Setup dependencies.
    $this->paths = $paths ? : new \SplPriorityQueue((array) $paths);

    $this->registerPaths(false);
}

$paths phpdoc 上是被定義成 SplPriorityQueue 物件 但是在建構子會被強制轉型成 array 可能要討論一下 $paths 明確的定義 是否只允許 SplPriorityQueuenull 兩種型態

另外一點是,若 $paths 不是帶 SplPriorityQueuenull 在進行 $this->registerPaths(false); 時會因為 $this->paths 不是 SplPriorityQueue 而出錯

asika32764 commented 9 years ago

這就是要寫 test 的原因了,因為實際上在現有系統中沒有出現需要用到這個參數的case,所以這個漏洞從來沒發生過問題。

這邊的設計邏輯應該是這樣

/**
 * Constructor.
 *
 * @param string             $name  The instance name.
 * @param \SplPriorityQueue|array  $paths Paths to scan assets.
 */
public function __construct($name = 'windwalker', $paths = null)
{
    $this->name = $name;

    // Setup dependencies.
    $this->paths = $paths instanceof \SplPriorityQueue ? $paths : new \SplPriorityQueue((array) $paths);

    $this->registerPaths(false);
}

這樣就不會有問題了

asika32764 commented 8 years ago

032246572137dd2fefb8a1d4ac7e1c03aac7e4f7