Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like CI_Security often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use CI_Security, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
49 | class CI_Security { |
||
50 | |||
51 | /** |
||
52 | * List of sanitize filename strings |
||
53 | * |
||
54 | * @var array |
||
55 | */ |
||
56 | public $filename_bad_chars = array( |
||
57 | '../', '<!--', '-->', '<', '>', |
||
58 | "'", '"', '&', '$', '#', |
||
59 | '{', '}', '[', ']', '=', |
||
60 | ';', '?', '%20', '%22', |
||
61 | '%3c', // < |
||
62 | '%253c', // < |
||
63 | '%3e', // > |
||
64 | '%0e', // > |
||
65 | '%28', // ( |
||
66 | '%29', // ) |
||
67 | '%2528', // ( |
||
68 | '%26', // & |
||
69 | '%24', // $ |
||
70 | '%3f', // ? |
||
71 | '%3b', // ; |
||
72 | '%3d' // = |
||
73 | ); |
||
74 | |||
75 | /** |
||
76 | * Character set |
||
77 | * |
||
78 | * Will be overridden by the constructor. |
||
79 | * |
||
80 | * @var string |
||
81 | */ |
||
82 | public $charset = 'UTF-8'; |
||
83 | |||
84 | /** |
||
85 | * XSS Hash |
||
86 | * |
||
87 | * Random Hash for protecting URLs. |
||
88 | * |
||
89 | * @var string |
||
90 | */ |
||
91 | protected $_xss_hash; |
||
92 | |||
93 | /** |
||
94 | * CSRF Hash |
||
95 | * |
||
96 | * Random hash for Cross Site Request Forgery protection cookie |
||
97 | * |
||
98 | * @var string |
||
99 | */ |
||
100 | protected $_csrf_hash; |
||
101 | |||
102 | /** |
||
103 | * CSRF Expire time |
||
104 | * |
||
105 | * Expiration time for Cross Site Request Forgery protection cookie. |
||
106 | * Defaults to two hours (in seconds). |
||
107 | * |
||
108 | * @var int |
||
109 | */ |
||
110 | protected $_csrf_expire = 7200; |
||
111 | |||
112 | /** |
||
113 | * CSRF Token name |
||
114 | * |
||
115 | * Token name for Cross Site Request Forgery protection cookie. |
||
116 | * |
||
117 | * @var string |
||
118 | */ |
||
119 | protected $_csrf_token_name = 'ci_csrf_token'; |
||
120 | |||
121 | /** |
||
122 | * CSRF Cookie name |
||
123 | * |
||
124 | * Cookie name for Cross Site Request Forgery protection cookie. |
||
125 | * |
||
126 | * @var string |
||
127 | */ |
||
128 | protected $_csrf_cookie_name = 'ci_csrf_token'; |
||
129 | |||
130 | /** |
||
131 | * List of never allowed strings |
||
132 | * |
||
133 | * @var array |
||
134 | */ |
||
135 | protected $_never_allowed_str = array( |
||
136 | 'document.cookie' => '[removed]', |
||
137 | 'document.write' => '[removed]', |
||
138 | '.parentNode' => '[removed]', |
||
139 | '.innerHTML' => '[removed]', |
||
140 | '-moz-binding' => '[removed]', |
||
141 | '<!--' => '<!--', |
||
142 | '-->' => '-->', |
||
143 | '<![CDATA[' => '<![CDATA[', |
||
144 | '<comment>' => '<comment>' |
||
145 | ); |
||
146 | |||
147 | /** |
||
148 | * List of never allowed regex replacements |
||
149 | * |
||
150 | * @var array |
||
151 | */ |
||
152 | protected $_never_allowed_regex = array( |
||
153 | 'javascript\s*:', |
||
154 | '(document|(document\.)?window)\.(location|on\w*)', |
||
155 | 'expression\s*(\(|&\#40;)', // CSS and IE |
||
156 | 'vbscript\s*:', // IE, surprise! |
||
157 | 'wscript\s*:', // IE |
||
158 | 'jscript\s*:', // IE |
||
159 | 'vbs\s*:', // IE |
||
160 | 'Redirect\s+30\d', |
||
161 | "([\"'])?data\s*:[^\\1]*?base64[^\\1]*?,[^\\1]*?\\1?" |
||
162 | ); |
||
163 | |||
164 | /** |
||
165 | * Class constructor |
||
166 | * |
||
167 | * @return void |
||
168 | */ |
||
169 | public function __construct() |
||
197 | |||
198 | // -------------------------------------------------------------------- |
||
199 | |||
200 | /** |
||
201 | * CSRF Verify |
||
202 | * |
||
203 | * @return CI_Security |
||
204 | */ |
||
205 | public function csrf_verify() |
||
206 | { |
||
207 | // If it's not a POST request we will set the CSRF cookie |
||
208 | if (strtoupper($_SERVER['REQUEST_METHOD']) !== 'POST') |
||
209 | { |
||
210 | return $this->csrf_set_cookie(); |
||
211 | } |
||
212 | |||
213 | // Check if URI has been whitelisted from CSRF checks |
||
214 | if ($exclude_uris = config_item('csrf_exclude_uris')) |
||
215 | { |
||
216 | $uri = load_class('URI', 'core'); |
||
217 | foreach ($exclude_uris as $excluded) |
||
218 | { |
||
219 | if (preg_match('#^'.$excluded.'$#i'.(UTF8_ENABLED ? 'u' : ''), $uri->uri_string())) |
||
220 | { |
||
221 | return $this; |
||
222 | } |
||
223 | } |
||
224 | } |
||
225 | |||
226 | // Do the tokens exist in both the _POST and _COOKIE arrays? |
||
227 | if ( ! isset($_POST[$this->_csrf_token_name], $_COOKIE[$this->_csrf_cookie_name]) |
||
228 | OR $_POST[$this->_csrf_token_name] !== $_COOKIE[$this->_csrf_cookie_name]) // Do the tokens match? |
||
229 | { |
||
230 | $this->csrf_show_error(); |
||
231 | } |
||
232 | |||
233 | // We kill this since we're done and we don't want to polute the _POST array |
||
234 | unset($_POST[$this->_csrf_token_name]); |
||
235 | |||
236 | // Regenerate on every submission? |
||
237 | if (config_item('csrf_regenerate')) |
||
238 | { |
||
239 | // Nothing should last forever |
||
240 | unset($_COOKIE[$this->_csrf_cookie_name]); |
||
241 | $this->_csrf_hash = NULL; |
||
242 | } |
||
243 | |||
244 | $this->_csrf_set_hash(); |
||
245 | $this->csrf_set_cookie(); |
||
246 | |||
247 | log_message('info', 'CSRF token verified'); |
||
248 | return $this; |
||
249 | } |
||
250 | |||
251 | // -------------------------------------------------------------------- |
||
252 | |||
253 | /** |
||
254 | * CSRF Set Cookie |
||
255 | * |
||
256 | * @codeCoverageIgnore |
||
257 | * @return CI_Security |
||
258 | */ |
||
259 | public function csrf_set_cookie() |
||
260 | { |
||
261 | $expire = time() + $this->_csrf_expire; |
||
262 | $secure_cookie = (bool) config_item('cookie_secure'); |
||
263 | |||
264 | if ($secure_cookie && ! is_https()) |
||
265 | { |
||
266 | return FALSE; |
||
267 | } |
||
268 | |||
269 | setcookie( |
||
270 | $this->_csrf_cookie_name, |
||
271 | $this->_csrf_hash, |
||
272 | $expire, |
||
273 | config_item('cookie_path'), |
||
274 | config_item('cookie_domain'), |
||
275 | $secure_cookie, |
||
276 | config_item('cookie_httponly') |
||
277 | ); |
||
278 | log_message('info', 'CSRF cookie sent'); |
||
279 | |||
280 | return $this; |
||
281 | } |
||
282 | |||
283 | // -------------------------------------------------------------------- |
||
284 | |||
285 | /** |
||
286 | * Show CSRF Error |
||
287 | * |
||
288 | * @return void |
||
289 | */ |
||
290 | public function csrf_show_error() |
||
291 | { |
||
292 | show_error('The action you have requested is not allowed.', 403); |
||
293 | } |
||
294 | |||
295 | // -------------------------------------------------------------------- |
||
296 | |||
297 | /** |
||
298 | * Get CSRF Hash |
||
299 | * |
||
300 | * @see CI_Security::$_csrf_hash |
||
301 | * @return string CSRF hash |
||
302 | */ |
||
303 | public function get_csrf_hash() |
||
304 | { |
||
305 | return $this->_csrf_hash; |
||
306 | } |
||
307 | |||
308 | // -------------------------------------------------------------------- |
||
309 | |||
310 | /** |
||
311 | * Get CSRF Token Name |
||
312 | * |
||
313 | * @see CI_Security::$_csrf_token_name |
||
314 | * @return string CSRF token name |
||
315 | */ |
||
316 | public function get_csrf_token_name() |
||
317 | { |
||
318 | return $this->_csrf_token_name; |
||
319 | } |
||
320 | |||
321 | // -------------------------------------------------------------------- |
||
322 | |||
323 | /** |
||
324 | * XSS Clean |
||
325 | * |
||
326 | * Sanitizes data so that Cross Site Scripting Hacks can be |
||
327 | * prevented. This method does a fair amount of work but |
||
328 | * it is extremely thorough, designed to prevent even the |
||
329 | * most obscure XSS attempts. Nothing is ever 100% foolproof, |
||
330 | * of course, but I haven't been able to get anything passed |
||
331 | * the filter. |
||
332 | * |
||
333 | * Note: Should only be used to deal with data upon submission. |
||
334 | * It's not something that should be used for general |
||
335 | * runtime processing. |
||
336 | * |
||
337 | * @link http://channel.bitflux.ch/wiki/XSS_Prevention |
||
338 | * Based in part on some code and ideas from Bitflux. |
||
339 | * |
||
340 | * @link http://ha.ckers.org/xss.html |
||
341 | * To help develop this script I used this great list of |
||
342 | * vulnerabilities along with a few other hacks I've |
||
343 | * harvested from examining vulnerabilities in other programs. |
||
344 | * |
||
345 | * @param string|string[] $str Input data |
||
346 | * @param bool $is_image Whether the input is an image |
||
347 | * @return string |
||
348 | */ |
||
349 | public function xss_clean($str, $is_image = FALSE) |
||
557 | |||
558 | // -------------------------------------------------------------------- |
||
559 | |||
560 | /** |
||
561 | * XSS Hash |
||
562 | * |
||
563 | * Generates the XSS hash if needed and returns it. |
||
564 | * |
||
565 | * @see CI_Security::$_xss_hash |
||
566 | * @return string XSS hash |
||
567 | */ |
||
568 | public function xss_hash() |
||
580 | |||
581 | // -------------------------------------------------------------------- |
||
582 | |||
583 | /** |
||
584 | * Get random bytes |
||
585 | * |
||
586 | * @param int $length Output length |
||
587 | * @return string |
||
588 | */ |
||
589 | public function get_random_bytes($length) |
||
638 | |||
639 | // -------------------------------------------------------------------- |
||
640 | |||
641 | /** |
||
642 | * HTML Entities Decode |
||
643 | * |
||
644 | * A replacement for html_entity_decode() |
||
645 | * |
||
646 | * The reason we are not using html_entity_decode() by itself is because |
||
647 | * while it is not technically correct to leave out the semicolon |
||
648 | * at the end of an entity most browsers will still interpret the entity |
||
649 | * correctly. html_entity_decode() does not convert entities without |
||
650 | * semicolons, so we are left with our own little solution here. Bummer. |
||
651 | * |
||
652 | * @link http://php.net/html-entity-decode |
||
653 | * |
||
654 | * @param string $str Input |
||
655 | * @param string $charset Character set |
||
656 | * @return string |
||
657 | */ |
||
658 | public function entity_decode($str, $charset = NULL) |
||
723 | |||
724 | // -------------------------------------------------------------------- |
||
725 | |||
726 | /** |
||
727 | * Sanitize Filename |
||
728 | * |
||
729 | * @param string $str Input file name |
||
730 | * @param bool $relative_path Whether to preserve paths |
||
731 | * @return string |
||
732 | */ |
||
733 | public function sanitize_filename($str, $relative_path = FALSE) |
||
754 | |||
755 | // ---------------------------------------------------------------- |
||
756 | |||
757 | /** |
||
758 | * Strip Image Tags |
||
759 | * |
||
760 | * @param string $str |
||
761 | * @return string |
||
762 | */ |
||
763 | public function strip_image_tags($str) |
||
767 | |||
768 | // ---------------------------------------------------------------- |
||
769 | |||
770 | /** |
||
771 | * Compact Exploded Words |
||
772 | * |
||
773 | * Callback method for xss_clean() to remove whitespace from |
||
774 | * things like 'j a v a s c r i p t'. |
||
775 | * |
||
776 | * @used-by CI_Security::xss_clean() |
||
777 | * @param array $matches |
||
778 | * @return string |
||
779 | */ |
||
780 | protected function _compact_exploded_words($matches) |
||
784 | |||
785 | // -------------------------------------------------------------------- |
||
786 | |||
787 | /** |
||
788 | * Sanitize Naughty HTML |
||
789 | * |
||
790 | * Callback method for xss_clean() to remove naughty HTML elements. |
||
791 | * |
||
792 | * @used-by CI_Security::xss_clean() |
||
793 | * @param array $matches |
||
794 | * @return string |
||
795 | */ |
||
796 | protected function _sanitize_naughty_html($matches) |
||
875 | |||
876 | // -------------------------------------------------------------------- |
||
877 | |||
878 | /** |
||
879 | * JS Link Removal |
||
880 | * |
||
881 | * Callback method for xss_clean() to sanitize links. |
||
882 | * |
||
883 | * This limits the PCRE backtracks, making it more performance friendly |
||
884 | * and prevents PREG_BACKTRACK_LIMIT_ERROR from being triggered in |
||
885 | * PHP 5.2+ on link-heavy strings. |
||
886 | * |
||
887 | * @used-by CI_Security::xss_clean() |
||
888 | * @param array $match |
||
889 | * @return string |
||
890 | */ |
||
891 | View Code Duplication | protected function _js_link_removal($match) |
|
903 | |||
904 | // -------------------------------------------------------------------- |
||
905 | |||
906 | /** |
||
907 | * JS Image Removal |
||
908 | * |
||
909 | * Callback method for xss_clean() to sanitize image tags. |
||
910 | * |
||
911 | * This limits the PCRE backtracks, making it more performance friendly |
||
912 | * and prevents PREG_BACKTRACK_LIMIT_ERROR from being triggered in |
||
913 | * PHP 5.2+ on image tag heavy strings. |
||
914 | * |
||
915 | * @used-by CI_Security::xss_clean() |
||
916 | * @param array $match |
||
917 | * @return string |
||
918 | */ |
||
919 | View Code Duplication | protected function _js_img_removal($match) |
|
931 | |||
932 | // -------------------------------------------------------------------- |
||
933 | |||
934 | /** |
||
935 | * Attribute Conversion |
||
936 | * |
||
937 | * @used-by CI_Security::xss_clean() |
||
938 | * @param array $match |
||
939 | * @return string |
||
940 | */ |
||
941 | protected function _convert_attribute($match) |
||
945 | |||
946 | // -------------------------------------------------------------------- |
||
947 | |||
948 | /** |
||
949 | * Filter Attributes |
||
950 | * |
||
951 | * Filters tag attributes for consistency and safety. |
||
952 | * |
||
953 | * @used-by CI_Security::_js_img_removal() |
||
954 | * @used-by CI_Security::_js_link_removal() |
||
955 | * @param string $str |
||
956 | * @return string |
||
957 | */ |
||
958 | protected function _filter_attributes($str) |
||
971 | |||
972 | // -------------------------------------------------------------------- |
||
973 | |||
974 | /** |
||
975 | * HTML Entity Decode Callback |
||
976 | * |
||
977 | * @used-by CI_Security::xss_clean() |
||
978 | * @param array $match |
||
979 | * @return string |
||
980 | */ |
||
981 | protected function _decode_entity($match) |
||
994 | |||
995 | // -------------------------------------------------------------------- |
||
996 | |||
997 | /** |
||
998 | * Do Never Allowed |
||
999 | * |
||
1000 | * @used-by CI_Security::xss_clean() |
||
1001 | * @param string |
||
1002 | * @return string |
||
1003 | */ |
||
1004 | protected function _do_never_allowed($str) |
||
1015 | |||
1016 | // -------------------------------------------------------------------- |
||
1017 | |||
1018 | /** |
||
1019 | * Set CSRF Hash and Cookie |
||
1020 | * |
||
1021 | * @return string |
||
1022 | */ |
||
1023 | protected function _csrf_set_hash() |
||
1024 | { |
||
1025 | if ($this->_csrf_hash === NULL) |
||
1026 | { |
||
1027 | // If the cookie exists we will use its value. |
||
1028 | // We don't necessarily want to regenerate it with |
||
1029 | // each page load since a page could contain embedded |
||
1030 | // sub-pages causing this feature to fail |
||
1031 | if (isset($_COOKIE[$this->_csrf_cookie_name]) && is_string($_COOKIE[$this->_csrf_cookie_name]) |
||
1032 | && preg_match('#^[0-9a-f]{32}$#iS', $_COOKIE[$this->_csrf_cookie_name]) === 1) |
||
1033 | { |
||
1034 | return $this->_csrf_hash = $_COOKIE[$this->_csrf_cookie_name]; |
||
1035 | } |
||
1036 | |||
1037 | $rand = $this->get_random_bytes(16); |
||
1038 | $this->_csrf_hash = ($rand === FALSE) |
||
1039 | ? md5(uniqid(mt_rand(), TRUE)) |
||
1040 | : bin2hex($rand); |
||
1041 | } |
||
1042 | |||
1043 | return $this->_csrf_hash; |
||
1044 | } |
||
1045 | |||
1046 | } |
||
1047 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.