Passed
Push — master ( c0a3a7...3b84a4 )
by Jeroen
58:51
created

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

loose comparison of strings.

Best Practice Bug Major
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 by
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