mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-09-13 05:47:23 -04:00
fix(validation): allow Windows sendmail paths, tighten shell metachar exclusions Broaden PLAIN_FILESYSTEM_PATH_STRICT to accept real-world sendmail formats while blocking command injection characters not needed in valid paths. - OSPOSRules.php: allow space, colon, backslash for Windows paths (e.g. C:\wamp64\...) and trailing args (-t -i); still excludes ampersand, backtick, subshell, redirect, and cmd.exe metacharacters - OSPOSRulesTest.php: add cases for Windows paths, trailing args, and injection payloads - Remove 7 ConfigTest assertions that expected metacharacter rejection; add acceptance test for sendmail path with trailing args i18n(lang): expand mailpath_invalid message across all locales - Fill previously empty mailpath_invalid keys across all locales - Update existing translations (de-CH, de-DE, es-ES, es-MX, fr, nl-BE, nl-NL) to reflect newly allowed characters; nl locales corrected from English loanwords to proper Dutch terms - Add missing key to ckb/Config.php docs: remove security advisory IDs from public-facing files - AGENTS.md: extend no-advisory-ID rule to documentation and URLs - INSTALL.md: drop GHSA reference and advisory link from Host Header Injection guidance; rationale and fix instructions remain intact Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
190 lines
7.1 KiB
PHP
190 lines
7.1 KiB
PHP
<?php
|
|
|
|
namespace Tests\Config\Validation;
|
|
|
|
use App\Config\Validation\OSPOSRules;
|
|
use App\Models\Employee;
|
|
use CodeIgniter\Config\Factories;
|
|
use CodeIgniter\HTTP\IncomingRequest;
|
|
use CodeIgniter\Test\CIUnitTestCase;
|
|
use Config\OSPOS;
|
|
use Config\Services;
|
|
use PHPUnit\Framework\MockObject\MockObject;
|
|
|
|
/**
|
|
* Regression tests for the login validation flow in OSPOSRules::login_check().
|
|
*
|
|
* Guards the fix from commit 5dea748b0: gcaptcha must be validated before
|
|
* Employee::login() is attempted, and the error contract must be preserved.
|
|
*/
|
|
class OSPOSRulesTest extends CIUnitTestCase
|
|
{
|
|
protected function tearDown(): void
|
|
{
|
|
Factories::reset();
|
|
Services::reset();
|
|
|
|
parent::tearDown();
|
|
}
|
|
|
|
public function testFailedCaptchaReturnsWithoutCallingLogin(): void
|
|
{
|
|
$this->injectSettings(['gcaptcha_enable' => '1', 'gcaptcha_secret_key' => 'test-key']);
|
|
$this->injectRequest();
|
|
$employee = $this->injectEmployeeMock();
|
|
$employee->expects($this->never())->method('login');
|
|
|
|
$rules = $this->rulesWithCaptchaResult(false);
|
|
$error = null;
|
|
|
|
$result = $rules->login_check('admin', 'username,password', ['password' => 'secret'], $error);
|
|
|
|
$this->assertFalse($result);
|
|
$this->assertSame(lang('Login.invalid_gcaptcha'), $error);
|
|
}
|
|
|
|
public function testSuccessfulCaptchaWithInvalidCredentialsReturnsLoginError(): void
|
|
{
|
|
$this->injectSettings(['gcaptcha_enable' => '1', 'gcaptcha_secret_key' => 'test-key']);
|
|
$this->injectRequest();
|
|
$employee = $this->injectEmployeeMock();
|
|
$employee->expects($this->once())
|
|
->method('login')
|
|
->with('admin', 'wrong-password')
|
|
->willReturn(false);
|
|
|
|
$rules = $this->rulesWithCaptchaResult(true);
|
|
$error = null;
|
|
|
|
$result = $rules->login_check('admin', 'username,password', ['password' => 'wrong-password'], $error);
|
|
|
|
$this->assertFalse($result);
|
|
$this->assertSame(lang('Login.invalid_username_and_password'), $error);
|
|
}
|
|
|
|
public function testCaptchaDisabledWithInvalidCredentialsReturnsLoginError(): void
|
|
{
|
|
$this->injectSettings([]);
|
|
$this->injectRequest();
|
|
$employee = $this->injectEmployeeMock();
|
|
$employee->expects($this->once())
|
|
->method('login')
|
|
->with('admin', 'wrong-password')
|
|
->willReturn(false);
|
|
|
|
$rules = new OSPOSRules();
|
|
$error = null;
|
|
|
|
$result = $rules->login_check('admin', 'username,password', ['password' => 'wrong-password'], $error);
|
|
|
|
$this->assertFalse($result);
|
|
$this->assertSame(lang('Login.invalid_username_and_password'), $error);
|
|
}
|
|
|
|
public function testSuccessfulCaptchaWithValidCredentialsPasses(): void
|
|
{
|
|
$this->injectSettings(['gcaptcha_enable' => '1', 'gcaptcha_secret_key' => 'test-key']);
|
|
$this->injectRequest();
|
|
$employee = $this->injectEmployeeMock();
|
|
$employee->expects($this->once())
|
|
->method('login')
|
|
->with('admin', 'correct-password')
|
|
->willReturn(true);
|
|
|
|
$rules = $this->rulesWithCaptchaResult(true);
|
|
$error = null;
|
|
|
|
$result = $rules->login_check('admin', 'username,password', ['password' => 'correct-password'], $error);
|
|
|
|
$this->assertTrue($result);
|
|
$this->assertNull($error);
|
|
}
|
|
|
|
/**
|
|
* @dataProvider validPathStrictProvider
|
|
*/
|
|
public function testValidPathStrict(string $candidate, bool $expected): void
|
|
{
|
|
$rules = new OSPOSRules();
|
|
|
|
$this->assertSame($expected, $rules->valid_path_strict($candidate));
|
|
}
|
|
|
|
public static function validPathStrictProvider(): array
|
|
{
|
|
return [
|
|
'plain sendmail path' => ['/usr/sbin/sendmail', true],
|
|
'plain php path' => ['/usr/bin/php', true],
|
|
'path with dash and underscore' => ['/opt/my-mail_bin/sendmail.exe', true],
|
|
'path with trailing args' => ['/usr/sbin/sendmail -t -i', true],
|
|
'windows path' => ['C:\wamp64\bin\sendmail\sendmail.exe', true],
|
|
'windows path with trailing args' => ['C:\wamp64\bin\sendmail\sendmail.exe -t -i', true],
|
|
'empty string' => ['', false],
|
|
'trailing newline bypass payload' => ["/usr/bin/php\n", false],
|
|
'trailing newline plus injected command' => ["/usr/bin/php\nid", false],
|
|
'embedded newline mid-string' => ["/usr/bin/php\n/bin/sh", false],
|
|
'semicolon injection' => ['/usr/bin/php;id', false],
|
|
'pipe injection' => ['/usr/bin/php|id', false],
|
|
'ampersand injection' => ['/usr/bin/php & id', false],
|
|
'backtick injection' => ['/usr/bin/php `id`', false],
|
|
'dollar subshell injection' => ['/usr/bin/php $(id)', false],
|
|
'quoted args' => ['/usr/bin/php -r "phpinfo();"', false],
|
|
'redirect injection' => ['/usr/bin/php > /tmp/out', false],
|
|
'cmd.exe env var expansion' => ['C:\path\sendmail.exe %COMSPEC%', false],
|
|
'cmd.exe escape char' => ['C:\path\sendmail.exe ^& calc', false],
|
|
'cmd.exe command chaining' => ['C:\path\sendmail.exe && calc', false],
|
|
];
|
|
}
|
|
|
|
private function injectSettings(array $settings): void
|
|
{
|
|
$ospos = new OSPOS();
|
|
$ospos->settings = $settings;
|
|
Factories::injectMock('config', OSPOS::class, $ospos);
|
|
}
|
|
|
|
private function injectRequest(): void
|
|
{
|
|
$request = $this->getMockBuilder(IncomingRequest::class)
|
|
->disableOriginalConstructor()
|
|
->onlyMethods(['getPost', 'getIPAddress', 'getLocale'])
|
|
->getMock();
|
|
$request->method('getPost')->willReturn('dummy-captcha-response');
|
|
$request->method('getLocale')->willReturn('en');
|
|
$request->method('getIPAddress')->willReturn('127.0.0.1');
|
|
Services::injectMock('request', $request);
|
|
}
|
|
|
|
private function injectEmployeeMock(): MockObject
|
|
{
|
|
$employee = $this->getMockBuilder(Employee::class)
|
|
->disableOriginalConstructor()
|
|
->onlyMethods(['login'])
|
|
->getMock();
|
|
Factories::injectMock('models', Employee::class, $employee);
|
|
|
|
return $employee;
|
|
}
|
|
|
|
/**
|
|
* Builds an OSPOSRules instance whose gcaptcha_check() returns a fixed
|
|
* result, so the captcha outcome can be simulated without network access.
|
|
*/
|
|
private function rulesWithCaptchaResult(bool $captcha_result): OSPOSRules
|
|
{
|
|
return new class ($captcha_result) extends OSPOSRules {
|
|
private bool $captcha_result;
|
|
|
|
public function __construct(bool $captcha_result)
|
|
{
|
|
$this->captcha_result = $captcha_result;
|
|
}
|
|
|
|
protected function gcaptcha_check($response): bool
|
|
{
|
|
return $this->captcha_result;
|
|
}
|
|
};
|
|
}
|
|
}
|