nextcloud / mail

💌 Mail app for Nextcloud
https://apps.nextcloud.com/apps/mail
GNU Affero General Public License v3.0
828 stars 256 forks source link

Undefined array key in DKIM validator #9213

Open Pammetje opened 7 months ago

Pammetje commented 7 months ago

Steps to reproduce

Use the mail app with a valid secure mailserver (configured with DKIM, SPF DMARC enabled)

Expected behavior

I recieve no error messeages in nexcloud.log

Actual behavior

generates an error in nextcloud.log

Mail app version

3.5.0

Mailserver or service

Postfix

Operating system

Ubuntu 22.04.3 LTS

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database

MySQL

Additional info

{"reqId":"XZbzoJYTj37YYgyQX9OO","level":3,"time":"2024-01-03T08:51:46+00:00","remoteAddr":"","user":"","app":"PHP","method":"GET","url":"/index .php/apps/mail/api/messages/10491/dkim","message":"Undefined array key \"t\" at /var/www/html/nextcloud/apps/mail/vendor/phpmailer/dkimvalidator/src/Validator.php#140","userAgent":"Mozilla/5 .0 (X11; CrOS x86_64 14541.0.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.36","version":"28.0.1.1","data":{"app":"PHP"}}

ChristophWurst commented 7 months ago

https://github.com/PHPMailer/DKIMValidator/issues/9?

Pammetje commented 7 months ago

@ChristophWurst That indeed seems to fix the issue Thank you very much :-)

-->

line: 144 (Validator.php): if ((int) $dkimTags['x'] < (int) $dkimTags['t']) { MUST BE: if ( isset($dkimTags['t']) && ((int) $dkimTags['x'] < (int) $dkimTags['t']) ) {

joshtrichards commented 7 months ago

@ChristophWurst Looks quiet over in that repo and I'm doubtful a PR will go anywhere. Think we should fork it or did you already have something else in mind?

ChristophWurst commented 7 months ago

Sending a patch upstream would still be nice. We can then make a copy into our repo and use something like https://packagist.org/packages/cweagans/composer-patches to avoid a full fork.

StinkyTACO commented 4 months ago

i made a pr hoping this will be fixed this is my first pr in the open source world

https://github.com/angrychimp/php-dkim/pull/14/commits/3d90eff0e420c0f42e2112cd7e7ba1de884a7d5f

dustbro commented 2 months ago

I've been getting the following error: Undefined array key 1 at /var/www/nextcloud/apps/mail/vendor/phpmailer/dkimvalidator/src/Validator.php#52

It looks like the explode function might not always return an array with two elements.

            //Split into tags
            $dkimTags = explode(';', $signatureToProcess);
            foreach ($dkimTags as $tagIndex => $tagContent) {
                [$tagName, $tagValue] = explode('=', trim($tagContent), 2);
                unset($dkimTags[$tagIndex]);
                if ($tagName === '') {
                    continue;
                }
                $dkimTags[$tagName] = $tagValue;
            }

Possible solution is to skip the current iteration of the loop if the $tagContent string doesn't contain the '=' character.

  //Split into tags
  $dkimTags = explode(';', $signatureToProcess);
  foreach ($dkimTags as $tagIndex => $tagContent) {
      $tagParts = explode('=', trim($tagContent), 2);
      if (count($tagParts) < 2) {
          continue;
      }
      [$tagName, $tagValue] = $tagParts;
      unset($dkimTags[$tagIndex]);
      if ($tagName === '') {
          continue;
      }
      $dkimTags[$tagName] = $tagValue;
  }

I'm gonna give this a go and see what happens.

dustbro commented 2 months ago

So far, so good.