diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 3ae230e6..b5ad90db 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -10,6 +10,11 @@ When submitting pull requests, the pull request should be made to the version yo
This code base is currently using PHP version 7.4. All files are required to be linted with PSR-12 standard. This repository will automatically check PRs for linting compliance.
+Whenever frontend JS limits the possible values of a user input, PHP must also do the same, and tests must be written.
+There's nothing stopping a user from making custom HTTP POST requests.
+This can be done in `webroot/panel/*.php` while parsing headers, or preferrably in `resources/lib/*.php`.
+For example, both frontend JS and in the `UnityUser` class make sure that a login shell contains only ASCII characters, and this is tested in `test/functional/LoginShellSetTest.php`.
+
## Development Environment
### Setting up your Environment
@@ -51,4 +56,4 @@ The following users are available for testing:
### Changes to Dev Environment
-Should the default schema of the web portal change, the `ldap/bootstrap.ldif` and `sql/bootstrap.sql` must be updated for the LDAP server and the MySQL server, respectively.
\ No newline at end of file
+Should the default schema of the web portal change, the `ldap/bootstrap.ldif` and `sql/bootstrap.sql` must be updated for the LDAP server and the MySQL server, respectively.
diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php
index ccf7fb9e..a2e839da 100644
--- a/resources/lib/UnityUser.php
+++ b/resources/lib/UnityUser.php
@@ -446,8 +446,16 @@ public function getSSHKeys($ignorecache = false)
*/
public function setLoginShell($shell, $operator = null, $send_mail = true)
{
- // FIXME throw error if shell is not ascii
// ldap schema syntax is "IA5 String (1.3.6.1.4.1.1466.115.121.1.26)"
+ if (!mb_check_encoding($shell, 'ASCII')) {
+ throw new Exception("non ascii characters are not allowed in a login shell!");
+ }
+ if ($shell != trim($shell)) {
+ throw new Exception("leading/trailing whitespace is not allowed in a login shell!");
+ }
+ if (empty($shell)) {
+ throw new Exception("login shell must not be empty!");
+ }
$ldapUser = $this->getLDAPUser();
if ($ldapUser->exists()) {
$ldapUser->setAttribute("loginshell", $shell);
diff --git a/test/functional/LoginShellSetTest.php b/test/functional/LoginShellSetTest.php
index 86ffbaea..e49adb5f 100644
--- a/test/functional/LoginShellSetTest.php
+++ b/test/functional/LoginShellSetTest.php
@@ -28,15 +28,22 @@ public static function getShells()
// phpcs:enable
}
+ private function isShellValid(string $shell)
+ {
+ return (
+ (mb_check_encoding($shell, 'ASCII')) &&
+ ($shell == trim($shell)) &&
+ (!empty($shell))
+ );
+ }
+
#[DataProvider("getShells")]
public function testSetLoginShellCustom(string $shell): void
{
global $USER;
- // FIXME add check to avoid warning from ldap_modify
- if (!mb_check_encoding($shell, 'ASCII')) {
- $this->expectException("Exception");
+ if (!$this->isShellValid($shell)) {
+ $this->expectException(Exception::class);
}
- // FIXME shell is not validated
post(
__DIR__ . "/../../webroot/panel/account.php",
["form_type" => "loginshell", "shellSelect" => "Custom", "shell" => $shell]
@@ -48,11 +55,9 @@ public function testSetLoginShellCustom(string $shell): void
public function testSetLoginShellSelect(string $shell): void
{
global $USER;
- // FIXME add check to avoid warning from ldap_modify
- if (!mb_check_encoding($shell, 'ASCII')) {
+ if (!$this->isShellValid($shell)) {
$this->expectException("Exception");
}
- // FIXME shell is not validated
post(
__DIR__ . "/../../webroot/panel/account.php",
["form_type" => "loginshell", "shellSelect" => $shell]
diff --git a/webroot/panel/account.php b/webroot/panel/account.php
index 9e6272da..a1ba6833 100644
--- a/webroot/panel/account.php
+++ b/webroot/panel/account.php
@@ -225,6 +225,7 @@
?>
+