[AVATAR] Ensure this Action stays secure

This commit is contained in:
Diogo Cordeiro 2020-06-27 22:39:09 +01:00 committed by Diogo Peralta Cordeiro
parent ff75bb796b
commit f93d8694c3
3 changed files with 31 additions and 27 deletions

View File

@ -173,7 +173,7 @@ class AttachmentAction extends ManagedAction
* @return string etag http header * @return string etag http header
* @throws ServerException * @throws ServerException
*/ */
public function etag(): string public function etag(): ?string
{ {
if (common_config('site', 'use_x_sendfile')) { if (common_config('site', 'use_x_sendfile')) {
return null; return null;
@ -202,5 +202,4 @@ class AttachmentAction extends ManagedAction
return null; return null;
} }
} }
} }

View File

@ -15,19 +15,17 @@
// along with GNU social. If not, see <http://www.gnu.org/licenses/>. // along with GNU social. If not, see <http://www.gnu.org/licenses/>.
/** /**
* Retrieve user avatar by nickname action class. * Retrieve user avatar by filename action class.
* *
* @category Action * @category Action
* @package GNUsocial * @package GNUsocial
* *
* @license http://www.fsf.org/licensing/licenses/agpl.html AGPLv3 * @license http://www.fsf.org/licensing/licenses/agpl.html AGPLv3
*/ */
if (!defined('GNUSOCIAL')) { defined('GNUSOCIAL') || die;
exit(1);
}
/** /**
* Retrieve user avatar by nickname action class. * Retrieve user avatar by filename action class.
* *
* @category Action * @category Action
* @package GNUsocial * @package GNUsocial
@ -42,14 +40,21 @@ if (!defined('GNUSOCIAL')) {
*/ */
class AvatarAction extends Action class AvatarAction extends Action
{ {
public $filename; public $filename = null;
public $filepath = null;
public $mimetype = null;
protected function prepare(array $args = []) protected function prepare(array $args = [])
{ {
parent::prepare($args); parent::prepare($args);
if (empty($this->filename = $this->trimmed('file'))) { $this->filename = File::tryFilename($this->trimmed('file'));
$this->filepath = File::path($this->filename, common_config('avatar', 'dir'), false);
if (!file_exists($this->filepath)) {
// TRANS: Client error displayed trying to get a non-existing avatar. // TRANS: Client error displayed trying to get a non-existing avatar.
$this->clientError(_m('No such avatar.'), 404); $this->clientError(_m('No such avatar.'), 404);
} }
$this->mimetype = (new ImageFile(-1, $this->filepath))->mimetype;
return true; return true;
} }
@ -57,17 +62,6 @@ class AvatarAction extends Action
{ {
parent::handle(); parent::handle();
if (is_string($srv = common_config('avatar', 'server')) && $srv != '') { common_send_file($this->filepath, $this->mimetype, $this->filename, 'inline');
common_redirect(Avatar::url($this->filename), 302);
} else {
$filepath = common_config('avatar', 'dir') . $this->filename;
$info = @getimagesize($filepath);
if ($info !== false) {
common_send_file($filepath, $info['mime'], $this->filename, 'inline');
} else {
throw new UnsupportedMediaException(_m("Avatar is not an image."));
}
}
return true;
} }
} }

View File

@ -413,7 +413,7 @@ class File extends Managed_DataObject
/** /**
* Validation for as-saved base filenames * Validation for as-saved base filenames
* @param $filename * @param mixed $filename
* @return false|int * @return false|int
*/ */
public static function validFilename($filename) public static function validFilename($filename)
@ -421,7 +421,12 @@ class File extends Managed_DataObject
return preg_match('/^[A-Za-z0-9._-]+$/', $filename); return preg_match('/^[A-Za-z0-9._-]+$/', $filename);
} }
public static function tryFilename($filename) /**
* @param mixed $filename
* @return string
* @throws InvalidFilenameException
*/
public static function tryFilename($filename): string
{ {
if (!self::validFilename($filename)) { if (!self::validFilename($filename)) {
throw new InvalidFilenameException($filename); throw new InvalidFilenameException($filename);
@ -431,16 +436,22 @@ class File extends Managed_DataObject
} }
/** /**
* @param $filename * Construct a path
*
* @param mixed $filename Will be tested by tryFilename
* @param string|null $dir Attachments directory by default
* @param bool $test_filename
* @return string * @return string
* @throws InvalidFilenameException * @throws InvalidFilenameException
* @throws ServerException * @throws ServerException
*/ */
public static function path($filename) public static function path($filename, ?string $dir = null, bool $test_filename = true)
{ {
self::tryFilename($filename); if ($test_filename) {
self::tryFilename($filename);
}
$dir = common_config('attachments', 'dir'); $dir = $dir ?? common_config('attachments', 'dir');
if (!in_array($dir[mb_strlen($dir)-1], ['/', '\\'])) { if (!in_array($dir[mb_strlen($dir)-1], ['/', '\\'])) {
$dir .= DIRECTORY_SEPARATOR; $dir .= DIRECTORY_SEPARATOR;