Issues in UrlSigner.php - All Issues - Inspection of "Merge pull request #11567 from mrclay/11563_routes" - Elgg/Elgg - Measure and Improve Code Quality continuously with Scrutinizer

Passed

Push — master ( c0a3a7...3b84a4 )

by Jeroen

created 2018-01-08 10:08 UTC

engine/classes/Elgg/Security/UrlSigner.php (1 issue)

Showing only issues like (Show All)

loose comparison of strings.

Best Practice Bug Major

Ismayil Khayredinov 1

<?php

namespace Elgg\Security;

/**
 * Component for creating signed URLs
 *
 * @access private
 */
class UrlSigner {

	const KEY_MAC = '__elgg_mac';
	const KEY_EXPIRES = '__elgg_exp';

	/**
	 * Normalizes and signs the URL with SHA256 HMAC key
	 *
	 * @note Signed URLs do not offer CSRF protection and should not be used instead of action tokens.
	 *
	 * @param string $url     URL to sign
	 * @param string $expires Expiration time
	 *                        Accepts a string suitable for strtotime()
	 *                        Falsey values indicate non-expiring URL
	 * @return string
	 * @throws \InvalidArgumentException
	 */
	public function sign($url, $expires = false) {
		$url = elgg_normalize_url($url);
		
		$parts = parse_url($url);

		if (isset($parts['query'])) {
			$query = elgg_parse_str($parts['query']);
		} else {
			$query = [];
		}

		if (isset($query[self::KEY_MAC])) {
			throw new \InvalidArgumentException('URL has already been signed');
		}

		if ($expires) {
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
			$query[self::KEY_EXPIRES] = strtotime($expires);
		}

		ksort($query);

		$parts['query'] = http_build_query($query);

		$url = elgg_http_build_url($parts, false);

		$token = elgg_build_hmac($url)->getToken();

		return elgg_http_add_url_query_elements($url, [
			self::KEY_MAC => $token,
		]);
	}

	/**
	 * Validates HMAC signature
	 *
	 * @param string $url URL to vlaidate
	 * @return bool
	 */
	public function isValid($url) {

		$parts = parse_url($url);

		if (isset($parts['query'])) {
			$query = elgg_parse_str($parts['query']);
		} else {
			$query = [];
		}
		
		if (!isset($query[self::KEY_MAC])) {
			// No signature found
			return false;
		}

		$token = $query[self::KEY_MAC];
		unset($query[self::KEY_MAC]);

		if (isset($query[self::KEY_EXPIRES]) && $query[self::KEY_EXPIRES] < time()) {
			// Signature has expired
			return false;
		}

		ksort($query);
		
		$parts['query'] = http_build_query($query);

		$url = elgg_http_build_url($parts, false);
		
		return elgg_build_hmac($url)->matchesToken($token);

	}
}


1		<?php
2
3		namespace Elgg\Security;
4
5		/**
6		* Component for creating signed URLs
7		*
8		* @access private
9		*/
10		class UrlSigner {
11
12		const KEY_MAC = '__elgg_mac';
13		const KEY_EXPIRES = '__elgg_exp';
14
15		/**
16		* Normalizes and signs the URL with SHA256 HMAC key
17		*
18		* @note Signed URLs do not offer CSRF protection and should not be used instead of action tokens.
19		*
20		* @param string $url URL to sign
21		* @param string $expires Expiration time
22		* Accepts a string suitable for strtotime()
23		* Falsey values indicate non-expiring URL
24		* @return string
25		* @throws \InvalidArgumentException
26		*/
27	5	public function sign($url, $expires = false) {
28	5	$url = elgg_normalize_url($url);
29
30	5	$parts = parse_url($url);
31
32	5	if (isset($parts['query'])) {
33	5	$query = elgg_parse_str($parts['query']);
34		} else {
35		$query = [];
36		}
37
38	5	if (isset($query[self::KEY_MAC])) {
39	1	throw new \InvalidArgumentException('URL has already been signed');
40		}
41
42	5	if ($expires) {
		0 ignored issues – show Bug Best Practice introduced 2017-10-16 06:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this The expression `$expires` of type `false\|string` is loosely compared to `true`; this is ambiguous if the string can be empty. You might want to explicitly use `!== false` instead. In PHP, under loose comparison (like `==`, or `!=`, or `switch` conditions), values of different types might be equal. For `string` values, the empty string `''` is a special case, in particular the following results might be unexpected: '' == false // true '' == null // true 'ab' == false // false 'ab' == null // false // It is often better to use strict comparison '' === false // false '' === null // false Loading history...
43	3	$query[self::KEY_EXPIRES] = strtotime($expires);
44		}
45
46	5	ksort($query);
47
48	5	$parts['query'] = http_build_query($query);
49
50	5	$url = elgg_http_build_url($parts, false);
51
52	5	$token = elgg_build_hmac($url)->getToken();
53
54	5	return elgg_http_add_url_query_elements($url, [
55	5	self::KEY_MAC => $token,
56		]);
57		}
58
59		/**
60		* Validates HMAC signature
61		*
62		* @param string $url URL to vlaidate
63		* @return bool
64		*/
65	4	public function isValid($url) {
66
67	4	$parts = parse_url($url);
68
69	4	if (isset($parts['query'])) {
70	4	$query = elgg_parse_str($parts['query']);
71		} else {
72		$query = [];
73		}
74
75	4	if (!isset($query[self::KEY_MAC])) {
76		// No signature found
77		return false;
78		}
79
80	4	$token = $query[self::KEY_MAC];
81	4	unset($query[self::KEY_MAC]);
82
83	4	if (isset($query[self::KEY_EXPIRES]) && $query[self::KEY_EXPIRES] < time()) {
84		// Signature has expired
85	1	return false;
86		}
87
88	4	ksort($query);
89
90	4	$parts['query'] = http_build_query($query);
91
92	4	$url = elgg_http_build_url($parts, false);
93
94	4	return elgg_build_hmac($url)->matchesToken($token);
95
96		}
97		}
98

Elgg / Elgg

Push — master ( c0a3a7...3b84a4 )

engine/classes/Elgg/Security/UrlSigner.php (1 issue)

Showing only issues like (Show All)

Introduced By

Duplication Side-by-Side

Filter issues like