Dereku/snappymail
0
0
Fork 0
mirror of https://github.com/the-djmaze/snappymail.git synced 2026-11-04 03:18:49 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

@ZaphodB fix(@security): S/MIME actions put the client's part id into IMAP FETCH as is

Browse source 
DoSMimeDecryptMessage and DoSMimeVerifyMessage built BODY.PEEK[<partId>]
(and [<partId>.MIME]) from the partId / sigPartId parameters without checking
them, so a value could close the bracket or the line and add IMAP commands of
its own. They run in the user's own IMAP session, so this is not cross-account,
but nothing in that session should be steerable by a request parameter.

Read both through smimePartIdParam(), which accepts only an IMAP section number
(1, 1.2.10) or TEXT and refuses anything else.

test/smime-partid.php checks valid ids pass and injection attempts, zero or
leading-zero sections, lowercase and other section keywords are refused.
This commit is contained in:
djmaze 2026-10-27 22:50:28 +01:00
parent a67c85885e
commit 099ee3b97e
2 changed files with 56 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

23
snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php
View file

@ -9,6 +9,7 @@ use MailSo\Imap\Enumerations\FetchType;
trait SMime
{
private $SMIME = null;
public function SMIME() : OpenSSL
{
if (!$this->SMIME) {
@ -84,7 +85,7 @@ trait SMime
{
$sFolderName = $this->GetActionParam('folder', '');
$iUid = (int) $this->GetActionParam('uid', 0);
$sPartId = $this->GetActionParam('partId', '');
$sPartId = $this->smimePartIdParam('partId');
$sCertificate = $this->GetActionParam('certificate', '');
$sPrivateKey = $this->GetActionParam('privateKey', '');
$oPassphrase = new \SnappyMail\SensitiveString($this->GetActionParam('passphrase', ''));
@ -137,7 +138,7 @@ trait SMime
public function DoSMimeVerifyMessage() : array
{
$sBody = $this->GetActionParam('bodyPart', '');
$sPartId = $this->GetActionParam('partId', '');
$sPartId = $this->smimePartIdParam('partId');
$bDetached = !empty($this->GetActionParam('detached', 0));
if (!$sBody && $sPartId) {
$iUid = (int) $this->GetActionParam('uid', 0);
@ -152,7 +153,7 @@ trait SMime
// Import the certificates automatically
$sBody = $this->GetActionParam('sigPart', '');
$sPartId = $this->GetActionParam('sigPartId', '') ?: $sPartId;
$sPartId = $this->smimePartIdParam('sigPartId') ?: $sPartId;
if (!$sBody && $sPartId && $oImapClient) {
$sBody = $oImapClient->Fetch(
[FetchType::BODY_PEEK.'['.$sPartId.']'],
@ -189,7 +190,7 @@ trait SMime
/*
$sBody = $this->GetActionParam('sigPart', '');
if (!$sBody) {
$sPartId = $this->GetActionParam('sigPartId', '') ?: $this->GetActionParam('partId', '');
$sPartId = $this->smimePartIdParam('sigPartId') ?: $this->smimePartIdParam('partId');
$this->initMailClientConnection();
$oImapClient = $this->ImapClient();
$oImapClient->FolderExamine($this->GetActionParam('folder', ''));
@ -212,4 +213,18 @@ trait SMime
return $this->DefaultResponse($certificates);
*/
}
/**
* A part id from the client goes into an IMAP FETCH section, BODY.PEEK[...],
* as is. Accept only a section number (1, 1.2.3) or TEXT, so the value cannot
* close the bracket or the line and add commands of its own.
*/
private function smimePartIdParam(string $sKey): string
{
$sPartId = \trim((string) $this->GetActionParam($sKey, ''));
if ('' !== $sPartId && !\preg_match('/^(TEXT|[1-9][0-9]*(\.[1-9][0-9]*)*)$/D', $sPartId)) {
throw new \RainLoop\Exceptions\ClientException(\RainLoop\Notifications::InvalidInputArgument);
}
return $sPartId;
}
}

37
test/smime-partid.php Normal file
View file

@ -0,0 +1,37 @@
<?php
// Run with: php test/smime-partid.php
// Enable SnappyMail Api and include index file
$_ENV['SNAPPYMAIL_INCLUDE_AS_API'] = true;
require __DIR__ . '/../index.php';
function check(bool $condition, string $message): void
{
if (!$condition) {
throw new RuntimeException($message);
}
}
$actions = (new ReflectionClass(\RainLoop\Actions::class))->newInstanceWithoutConstructor();
$params = new ReflectionProperty(\RainLoop\Actions::class, 'aCurrentActionParams');
$method = new ReflectionMethod(\RainLoop\Actions::class, 'smimePartIdParam');
$read = function (string $value) use ($actions, $params, $method) : ?string {
$params->setValue($actions, ['partId' => $value]);
try {
return $method->invoke($actions, 'partId');
} catch (\RainLoop\Exceptions\ClientException $e) {
return null;
}
};
foreach (['', '1', '2', '1.2', '1.2.10', 'TEXT'] as $ok) {
check($ok === $read($ok), "valid part id '{$ok}' refused");
}
foreach (["1]\r\nA1 LOGOUT\r\n", '1] BODY[HEADER', "1\r\nA1 LOGOUT", '1 2', '0', '1.0', '01', '1.', '.1', 'text', 'HEADER'] as $bad) {
check(null === $read($bad), 'part id '.json_encode($bad).' accepted');
}
// Surrounding whitespace is trimmed away, and the trimmed value is what is used.
check('1' === $read("1\n"), 'trailing newline not trimmed');
echo "smime-partid: ok\n";