mailin icon indicating copy to clipboard operation
mailin copied to clipboard

Attachments array keys & PHP

Open 2naive opened this issue 11 years ago • 10 comments

Hi!

There is a problem described below: http://stackoverflow.com/questions/68651/get-php-to-stop-replacing-characters-in-get-or-post-arrays http://ca.php.net/manual/en/language.variables.external.php#81080

e.g. if I have attachment file:

fileName":"Folder.jpg", "generatedFileName":"Folder.jpg"

on PHP side I'll get an array key

'Folder_jpg'

So, may be it's better to generate unique keys for attachments array OR generate "generatedFileName" to bypass that problem?

Thank you!

2naive avatar Jan 09 '15 05:01 2naive

When several attachements have the same fileName, generatedFileName will be different from fileName, in order to get a kind of 'unique' file name.

Anyway, I am sorry but I am not sure to understand clearly the question. Do you want to identify uniquely the fileNames or to retrieve the original fileName, replacing back underscores by dots? If the issue is uniquely identifying the attachments, you can use the contentId key, which will always be unique.

Flolagale avatar Jan 12 '15 09:01 Flolagale

Ok, I'll try explain again.

In example here: https://github.com/Flolagale/mailin/blob/master/README.md

You wrote:

  attachments: [{
      contentType: 'text/plain',
      fileName: 'dummyFile.txt',
      contentDisposition: 'attachment',
      transferEncoding: 'base64',
      generatedFileName: 'dummyFile.txt',
      contentId: '6e4a9c577e603de61e554abab84f6297@mailparser',
      checksum: 'e9fa6319356c536b962650eda9399a44',
      length: '28'
  }],
...
'dummyFile.txt': 'a-base64-encoded-string=='

So, in that case dummyFile.txt is a key of associative array when it's posted to .php webhook. In that case, if webhook will be PHP-script (I suppose It could be similar problems with other languages) you will see:

[dummyFile_txt] => 'a-base64-encoded-string=='

or

 [Телефонный_трафик_htm] => PGh0bWw+PGhlYWQ+DQo8bWV0YSBodHRwLWVxdWl2PSJjb250ZW50LXR5cGUiIGNvbnRlbnQ9InRleHQvaHRtbDsgY2hhcnNldD1VVEYtOCI+DQo8dGl0bGU+0KLQtdC70LXRhNC+
  • in my test case.

So all I'm saying is that it having filename as array key is unsafe in case wehook could be written in languages other than JS.

Yes, there is a workaround like:

function _fixed_name($name) {
    return str_replace(array(chr(32), chr(46), chr(91), chr(128), chr(129), chr(130), chr(131), chr(132), chr(133), chr(134), chr(135), chr(136), chr(137), chr(138), chr(139), chr(140), chr(141), chr(142), chr(143), chr(144), chr(145), chr(146), chr(147), chr(148), chr(149), chr(150), chr(151), chr(152), chr(153), chr(154), chr(155), chr(156), chr(157), chr(158), chr(159)), '_', $name);
}
  • but even in that case, if you have 2 different files in one message with, for example, such names: name .jpg AND name_.jpg

They all will be converted by this function to name__.jpg.

So, in that case I think that array key should be safe (e.g. base64), or just some unique id, which would also present in attachments[], but not a filename.

Or just change fileName to contentId for array key, e.g:

 '6e4a9c577e603de61e554abab84f6297@mailparser': 'a-base64-encoded-string=='

NOT

 'dummyFile.txt': 'a-base64-encoded-string=='

Hope I could explain it.

2naive avatar Jan 13 '15 04:01 2naive

Ok, you made an interesting point. Using the contentId as the key is a good idea, but it introduces some breaking changes to the API. I will implement it but I would like to make sure that it is absolutely necessary. Could you please answer to the 2 following questions to verify this?

  • For now, the attachment array key is supposed to be the generatedFileName. In your example, the filename is Телефонный_трафик_htm, what is the value of the associated generatedFileName?
  • The case that you described (2 attached files 'Телефонный_трафик_.htm' and 'Телефонный_трафик .htm') should this still lead to 2 different generatedFilenames. Could you try it and post the resulting generatedFileNames here?

Flolagale avatar Jan 13 '15 09:01 Flolagale

[attachments] => Array
    (
        [0] => stdClass Object
            (
                [contentType] => text/html
                [charset] => utf-8
                [fileName] => Телефонный трафик.htm
                [transferEncoding] => base64
                [contentDisposition] => attachment
                [generatedFileName] => Телефонный трафик.htm
                [contentId] => 8184317dea9324c49f3ada75a10fff4a@mailparser
                [checksum] => 6f34800999d4a4b504737f87488f9d0a
                [length] => 123797
            )

    )
... ...
[Телефонный_трафик_htm] => PGh0bWw+PGhlYWQ+DQo8bWV0YSBodHRwLWVxdWl...
  1. As I wrote:

    ...names: name .jpg AND name_.jpg They all will be converted by this function to name__.jpg.

  • first name is "name<SPACE>.jpg" and second is "name<UNDERSCORE>.jpg"

So, okay, I did the following:

 attachments:
   [ { contentType: 'text/plain',
       charset: 'utf-8',
       fileName: 'test.txt',
       contentDisposition: 'attachment',
       transferEncoding: 'base64',
       generatedFileName: 'test.txt',
       contentId: 'dd18bf3a8e0a2a3e53e2661c7fb53534@mailparser',
       checksum: 'c4ca4238a0b923820dcc509a6f75849b',
       length: 1 },
     { contentType: 'application/octet-stream',
       fileName: 'test txt',
       contentDisposition: 'attachment',
       transferEncoding: 'base64',
       generatedFileName: 'test txt',
       contentId: '62c97d2c2bdfba2320083770dfb45ec9@mailparser',
       checksum: 'c4ca4238a0b923820dcc509a6f75849b',
       length: 1 } ],
  • from mailing logs.

Got from PHP webhook dump:

array (
  'mailinMsg' => '{"html":"<div dir=\\"ltr\\"><div class=\\"gmail_default\\" style=\\"font-family:tahoma,sans-serif\\"><br clear=\\"all\\"></div><div><div class=\\"gmail_signature\\"><div><br></div><div><br></div><font face=\\"tahoma, sans-serif\\">С уважением,</font><div><font face=\\"tahoma, sans-serif\\">Александр</font></div></div></div>\\r\\n</div>\\r\\n","text":"С уважением,\\r\\n Александр\\r\\n","headers":{"received":["by mail-lb0-f173.google.com with SMTP id z12so3832792lbi.4 for <[email protected]>; Tue, 13 Jan 2015 09:36:48 -0800 (PST)","by 10.112.125.227 with HTTP; Tue, 13 Jan 2015 09:36:47 -0800 (PST)"],"dkim-signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=dYMvmrvXOgQGoMnYLjdafuYr13T28MPpE81CHhknhK4=; b=pILxOJ4uSbK+ZZCJjR6hjwQ63U5b0QLjg2hnKqjnCcIlKd6WtAJrn0X2YCfzhJ+D31 xOf2tiFuW4SN5FGy/HHuXQ20q41Nu5GBpNDjjetMOpDwLNVd5/s0M3Nd3DVK9TCPBLzm T6/emGi8S5C1gQvNXYsowzybUluFKIUyLR3QL1UMx0wYWJxTFnvycB5xg83w9rFiLycg tOTMYA5X3cfcNp6fUNd8XfLrzcW+29MoREU/ub5HSeqQ14mZPbb+b3y3zoscVGxf7tKu nCoLHKSo1N1N7lk2P8zy7e2QgJpbC4WBnWnh3VA9E11DqEuCRZQiKpt8ZkTgddkck3Ti oWQA==","mime-version":"1.0","x-received":"by 10.152.44.167 with SMTP id f7mr43671696lam.92.1421170608225; Tue, 13 Jan 2015 09:36:48 -0800 (PST)","date":"Tue, 13 Jan 2015 20:36:47 +0300","message-id":"<CAKyjARYsHu-GLbnXDtGhHRkLRk8CCEevbSoKxHnW7ydWYgefAw@mail.gmail.com>","subject":"test","from":"Александр  <[email protected]>","to":"[email protected]","content-type":"multipart/mixed; boundary=089e0158c40e44dccf050c8c1162"},"subject":"test","messageId":"CAKyjARYsHu-GLbnXDtGhHRkLRk8CCEevbSoKxHnW7ydWYgefAw@mail.gmail.com","priority":"normal","from":[{"address":"[email protected]","name":"Александр"}],"to":[{"address":"[email protected]","name":""}],"date":"2015-01-13T17:36:47.000Z","attachments":[{"contentType":"text/plain","charset":"utf-8","fileName":"test.txt","contentDisposition":"attachment","transferEncoding":"base64","generatedFileName":"test.txt","contentId":"dd18bf3a8e0a2a3e53e2661c7fb53534@mailparser","checksum":"c4ca4238a0b923820dcc509a6f75849b","length":1},{"contentType":"application/octet-stream","fileName":"test txt","contentDisposition":"attachment","transferEncoding":"base64","generatedFileName":"test txt","contentId":"62c97d2c2bdfba2320083770dfb45ec9@mailparser","checksum":"c4ca4238a0b923820dcc509a6f75849b","length":1}],"dkim":"pass","spf":"pass","spamScore":0,"language":"bulgarian","cc":[],"connection":{"from":"[email protected]","to":["[email protected]"],"remoteAddress":"209.85.217.173","authentication":{"username":false,"authenticated":false,"state":"NORMAL"},"id":"238dead6"},"envelopeFrom":[{"address":"[email protected]","name":""}],"envelopeTo":[{"address":"[email protected]","name":""}]}',
  'test_txt' => 'MQ==',
)

So you see just 1 file test_txt.

And I don't know for sure all symbols that could be replaced.

2naive avatar Jan 13 '15 17:01 2naive

Also in the same dump you may see the problem with envelopeFrom.

2naive avatar Jan 13 '15 17:01 2naive

Any news?

2naive avatar Jan 29 '15 20:01 2naive

My workaround isn't working with chr(128) - chr(159), because: _fixed_name('Телефонный траффик.htm') gives: Теле�_онн�_й_�_�_а�_ик_htm

2naive avatar Jan 29 '15 22:01 2naive

Sorry I have been very busy lately, I did not get the time to fix this. I think we will use the contentId as a key. I will keep you posted!

Flolagale avatar Feb 03 '15 10:02 Flolagale

ContentID could also have dots, as I understand. May be its better to add one more I'D or just use an iterator? or put content inside attachments array.

2naive avatar Feb 03 '15 10:02 2naive

Is this necessarily a security issue in PHP? You mention it being unsafe but is it not rather an inconvenience? I agree that the dot notation should probably be revised since even in JS it's not exactly semantic (. being the object/property separator) but just wanted to check that it isn't exactly an urgent security vulnerability?

Ghirro avatar Mar 18 '15 09:03 Ghirro