Skip to content

Commit cdff9ed

Browse files
kenjiskenjis
authored
Merge pull request #5227 from kenjis/add-filter-invalidChars
feat: add filter to check invalid chars in user input
2 parents 445cb65 + 9d7e038 commit cdff9ed

File tree

5 files changed

+310
-5
lines changed

5 files changed

+310
-5
lines changed

‎app/Config/Filters.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use CodeIgniter\Filters\CSRF;
77
use CodeIgniter\Filters\DebugToolbar;
88
use CodeIgniter\Filters\Honeypot;
9+
use CodeIgniter\Filters\InvalidChars;
910

1011
class Filters extends BaseConfig
1112
{
@@ -16,9 +17,10 @@ class Filters extends BaseConfig
1617
* @var array
1718
*/
1819
public $aliases = [
19-
'csrf' => CSRF::class,
20-
'toolbar' => DebugToolbar::class,
21-
'honeypot' => Honeypot::class,
20+
'csrf' => CSRF::class,
21+
'toolbar' => DebugToolbar::class,
22+
'honeypot' => Honeypot::class,
23+
'invalidchars' => InvalidChars::class,
2224
];
2325

2426
/**
@@ -31,6 +33,7 @@ class Filters extends BaseConfig
3133
'before' => [
3234
// 'honeypot',
3335
// 'csrf',
36+
// 'invalidchars',
3437
],
3538
'after' => [
3639
'toolbar',

‎system/Filters/InvalidChars.php

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace CodeIgniter\Filters;
13+
14+
use CodeIgniter\HTTP\RequestInterface;
15+
use CodeIgniter\HTTP\ResponseInterface;
16+
use CodeIgniter\Security\Exceptions\SecurityException;
17+
18+
/**
19+
* InvalidChars filter.
20+
*
21+
* Check if user input data ($_GET, $_POST, $_COOKIE, php://input) do not contain
22+
* invalid characters:
23+
* - invalid UTF-8 characters
24+
* - control characters except line break and tab code
25+
*/
26+
class InvalidChars implements FilterInterface
27+
{
28+
/**
29+
* Data source
30+
*
31+
* @var string
32+
*/
33+
protected $source;
34+
35+
/**
36+
* Regular expressions for valid control codes
37+
*
38+
* @var string
39+
*/
40+
protected $controlCodeRegex = '/\A[\r\n\t[:^cntrl:]]*\z/u';
41+
42+
/**
43+
* Check invalid characters.
44+
*
45+
* @param array|null $arguments
46+
*
47+
* @return void
48+
*/
49+
public function before(RequestInterface $request, $arguments = null)
50+
{
51+
if ($request->isCLI()) {
52+
return;
53+
}
54+
55+
$data = [
56+
'get' => $request->getGet(),
57+
'post' => $request->getPost(),
58+
'cookie' => $request->getCookie(),
59+
'rawInput' => $request->getRawInput(),
60+
];
61+
62+
foreach ($data as $source => $values) {
63+
$this->source = $source;
64+
$this->checkEncoding($values);
65+
$this->checkControl($values);
66+
}
67+
}
68+
69+
/**
70+
* We don't have anything to do here.
71+
*
72+
* @param array|null $arguments
73+
*
74+
* @return void
75+
*/
76+
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
77+
{
78+
}
79+
80+
/**
81+
* Check the character encoding is valid UTF-8.
82+
*
83+
* @param array|string $value
84+
*
85+
* @return array|string
86+
*/
87+
protected function checkEncoding($value)
88+
{
89+
if (is_array($value)) {
90+
array_map([$this, 'checkEncoding'], $value);
91+
92+
return $value;
93+
}
94+
95+
if (mb_check_encoding($value, 'UTF-8')) {
96+
return $value;
97+
}
98+
99+
throw SecurityException::forInvalidUTF8Chars($this->source, $value);
100+
}
101+
102+
/**
103+
* Check for the presence of control characters except line breaks and tabs.
104+
*
105+
* @param array|string $value
106+
*
107+
* @return array|string
108+
*/
109+
protected function checkControl($value)
110+
{
111+
if (is_array($value)) {
112+
array_map([$this, 'checkControl'], $value);
113+
114+
return $value;
115+
}
116+
117+
if (preg_match($this->controlCodeRegex, $value) === 1) {
118+
return $value;
119+
}
120+
121+
throw SecurityException::forInvalidControlChars($this->source, $value);
122+
}
123+
}

‎system/Security/Exceptions/SecurityException.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,22 @@ public static function forDisallowedAction()
2020
return new static(lang('Security.disallowedAction'), 403);
2121
}
2222

23+
public static function forInvalidUTF8Chars(string $source, string $string)
24+
{
25+
return new static(
26+
'Invalid UTF-8 characters in ' . $source . ': ' . $string,
27+
400
28+
);
29+
}
30+
31+
public static function forInvalidControlChars(string $source, string $string)
32+
{
33+
return new static(
34+
'Invalid Control characters in ' . $source . ': ' . $string,
35+
400
36+
);
37+
}
38+
2339
/**
2440
* @deprecated Use `CookieException::forInvalidSameSite()` instead.
2541
*

‎tests/system/Filters/InvalidCharsTest.php

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace CodeIgniter\Filters;
13+
14+
use CodeIgniter\HTTP\CLIRequest;
15+
use CodeIgniter\HTTP\IncomingRequest;
16+
use CodeIgniter\HTTP\URI;
17+
use CodeIgniter\HTTP\UserAgent;
18+
use CodeIgniter\Security\Exceptions\SecurityException;
19+
use CodeIgniter\Test\CIUnitTestCase;
20+
use CodeIgniter\Test\Mock\MockAppConfig;
21+
22+
/**
23+
* @internal
24+
*/
25+
final class InvalidCharsTest extends CIUnitTestCase
26+
{
27+
/**
28+
* @var InvalidChars
29+
*/
30+
private $invalidChars;
31+
32+
/**
33+
* @var IncomingRequest
34+
*/
35+
private $request;
36+
37+
protected function setUp(): void
38+
{
39+
parent::setUp();
40+
41+
$_GET = [];
42+
$_POST = [];
43+
$_COOKIE = [];
44+
45+
$this->request = $this->createRequest();
46+
$this->invalidChars = new InvalidChars();
47+
}
48+
49+
protected function tearDown(): void
50+
{
51+
parent::tearDown();
52+
53+
$_GET = [];
54+
$_POST = [];
55+
$_COOKIE = [];
56+
}
57+
58+
private function createRequest(): IncomingRequest
59+
{
60+
$config = new MockAppConfig();
61+
$uri = new URI();
62+
$userAgent = new UserAgent();
63+
$request = $this->getMockBuilder(IncomingRequest::class)
64+
->setConstructorArgs([$config, $uri, null, $userAgent])
65+
->onlyMethods(['isCLI'])
66+
->getMock();
67+
$request->method('isCLI')->willReturn(false);
68+
69+
return $request;
70+
}
71+
72+
/**
73+
* @doesNotPerformAssertions
74+
*/
75+
public function testBeforeDoNothingWhenCLIRequest()
76+
{
77+
$cliRequest = new CLIRequest(new MockAppConfig());
78+
79+
$this->invalidChars->before($cliRequest);
80+
}
81+
82+
/**
83+
* @doesNotPerformAssertions
84+
*/
85+
public function testBeforeValidString()
86+
{
87+
$_POST['val'] = [
88+
'valid string',
89+
];
90+
$_COOKIE['val'] = 'valid string';
91+
92+
$this->invalidChars->before($this->request);
93+
}
94+
95+
public function testBeforeInvalidUTF8StringCausesException()
96+
{
97+
$this->expectException(SecurityException::class);
98+
$this->expectExceptionMessage('Invalid UTF-8 characters in post:');
99+
100+
$sjisString = mb_convert_encoding('SJISの文字列です。', 'SJIS');
101+
$_POST['val'] = [
102+
'valid string',
103+
$sjisString,
104+
];
105+
106+
$this->invalidChars->before($this->request);
107+
}
108+
109+
public function testBeforeInvalidControlCharCausesException()
110+
{
111+
$this->expectException(SecurityException::class);
112+
$this->expectExceptionMessage('Invalid Control characters in cookie:');
113+
114+
$stringWithNullChar = "String contains null char and line break.\0\n";
115+
$_COOKIE['val'] = $stringWithNullChar;
116+
117+
$this->invalidChars->before($this->request);
118+
}
119+
120+
/**
121+
* @doesNotPerformAssertions
122+
*
123+
* @dataProvider stringWithLineBreakAndTabProvider
124+
*/
125+
public function testCheckControlStringWithLineBreakAndTabReturnsTheString(string $input)
126+
{
127+
$_GET['val'] = $input;
128+
129+
$this->invalidChars->before($this->request);
130+
}
131+
132+
public function stringWithLineBreakAndTabProvider()
133+
{
134+
yield from [
135+
["String contains \n line break."],
136+
["String contains \r line break."],
137+
["String contains \r\n line break."],
138+
["String contains \t tab."],
139+
["String contains \t and \r line \n break."],
140+
];
141+
}
142+
143+
/**
144+
* @dataProvider stringWithControlCharsProvider
145+
*/
146+
public function testCheckControlStringWithControlCharsCausesException(string $input)
147+
{
148+
$this->expectException(SecurityException::class);
149+
$this->expectExceptionMessage('Invalid Control characters in get:');
150+
151+
$_GET['val'] = $input;
152+
153+
$this->invalidChars->before($this->request);
154+
}
155+
156+
public function stringWithControlCharsProvider()
157+
{
158+
yield from [
159+
["String contains null char.\0"],
160+
["String contains null char and line break.\0\n"],
161+
];
162+
}
163+
}

‎user_guide_src/source/incoming/filters.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,6 @@ In this example, the array ``['dual', 'noreturn']`` will be passed in ``$argumen
193193
Provided Filters
194194
****************
195195

196-
Three filters are bundled with CodeIgniter4: ``Honeypot``, ``CSRF``, and ``DebugToolbar``.
196+
The filters bundled with CodeIgniter4 are: ``Honeypot``, ``CSRF``, ``DebugToolbar``, and ``InvalidChars``.
197197

198-
.. note:: The filters are executed in the declared order that is defined in the config file, but there is one exception to this and it concerns the ``DebugToolbar``, which is always executed last. This is because ``DebugToolbar`` should be able to register everything that happens in other filters.
198+
.. note:: The filters are executed in the order defined in the config file. However, if enabled, ``DebugToolbar`` is always executed last because it should be able to capture everything that happens in the other filters.

0 commit comments

Comments
 (0)