1. 03 Sep, 2014 3 commits
    • Alex Vandiver's avatar
      Standardize on the stricter Encode::encode("UTF-8", ...) everywhere · 1d18663b
      Alex Vandiver authored
      This is not only for code consistency, but also for consistency of
      output.  Encode::encode_utf8(...) is equivalent to
      Encode::encode("utf8",...) which is the non-"strict" form of UTF-8.
      Strict UTF-8 encoding differs in that (from `perldoc Encode`):
      
          ...its range is much narrower (0 ..  0x10_FFFF to cover only 21 bits
          instead of 32 or 64 bits) and some sequences are not allowed, like
          those used in surrogate pairs, the 31 non-character code points
          0xFDD0 .. 0xFDEF, the last two code points in any plane (0xXX_FFFE
          and 0xXX_FFFF), all non-shortest encodings, etc.
      
      RT deals with interchange with databases, email, and other systems.  In
      dealing with encodings, it should ensure that it does not produce byte
      sequences that are invalid according to official Unicode standards.
      1d18663b
    • Alex Vandiver's avatar
      Make RT::Action::SendEmail->SetHeader take characters, not bytes · 12c2671c
      Alex Vandiver authored
      This helper method is used in a number of places in
      RT::Action::SendEmail, often without remembering that it should be
      passed bytes, not characters.  Change it to always take characters, and
      modify the two callsite which (correctly) passed it bytes to no longer
      do so.
      12c2671c
    • Alex Vandiver's avatar
      Ensure all MIME::Entity headers are UTF-8 encoded bytes · 41d084f1
      Alex Vandiver authored
      Placing wide characters into MIME::Entity objects can lead to
      double-encoding, as discovered most recently in d469cacc.  Explicitly
      decode all headers as UTF-8 when retrieving them with ->get(), and
      encode them as UTF-8 before updating them with ->set() or ->replace().
      This also applies to headers passed to ->build().  The only exceptions
      to this are fixed strings in the source (which, in the absence of "use
      utf8", are always bytes).
      
      While the majority of these headers will never have wide characters in
      them, always decoding and encoding ensures the proper disipline to
      guarantee that strings with the "UTF8" flag do not get placed in a
      header, which can cause double-encoding.
      41d084f1
  2. 06 Jan, 2014 2 commits
  3. 27 Sep, 2013 2 commits
    • Alex Vandiver's avatar
      Ensure that the Subject header is not double-encoded · d469cacc
      Alex Vandiver authored
      In Encode 2.52 and below, `decode_utf8(...)` called on a string with the
      internal utf8" flag (i.e. containing characters, not bytes) was a no-op.
      Encode 2.53 changed this, for parity with `decode('utf8', ...)`
      
      The contents of the 'Subject' header when sending mail were usually
      bytes, as parsed by MIME-Tools.  As such, before adding the subject tag,
      they were decoded into characters.  However, in the case where no
      Subject was given by the template (for example, the "Transaction"
      template), RT generated a (wide-character) string for the Subject based
      on the ticket title, and inserted it into the MIME object.  Later,
      before the subject tag was added decode_utf8() was called on this wide
      string -- and prior to Encode 2.53, this was a no-op.
      
      Ensure that the values placed in the MIME header are always bytes, and
      not wide characters.  There are two changes to do this: first, ensure
      that the automatically-generated subject is encoded before being used;
      and second, that the string with the subject tag is re-encoded before
      the header is set to it.
      d469cacc
    • Alex Vandiver's avatar
      Ensure that the Subject header is not double-encoded · 8a30d764
      Alex Vandiver authored
      In Encode 2.52 and below, `decode_utf8(...)` called on a string with the
      internal utf8" flag (i.e. containing characters, not bytes) was a no-op.
      Encode 2.53 changed this, for parity with `decode('utf8', ...)`
      
      The contents of the 'Subject' header when sending mail were usually
      bytes, as parsed by MIME-Tools.  As such, before adding the subject tag,
      they were decoded into characters.  However, in the case where no
      Subject was given by the template (for example, the "Transaction"
      template), RT generated a (wide-character) string for the Subject based
      on the ticket title, and inserted it into the MIME object.  Later,
      before the subject tag was added decode_utf8() was called on this wide
      string -- and prior to Encode 2.53, this was a no-op.
      
      Ensure that the values placed in the MIME header are always bytes, and
      not wide characters.  There are two changes to do this: first, ensure
      that the automatically-generated subject is encoded before being used;
      and second, that the string with the subject tag is re-encoded before
      the header is set to it.
      8a30d764
  4. 04 Sep, 2013 1 commit
    • Kevin Falcone's avatar
      Prefix headers with X for RFC822 compliance · 4f34a6f3
      Kevin Falcone authored
      As documented in issues #20691, Amazon's smarthost service (SES) is
      requiring X prefixes, and while we're technically correct according to
      RFC2822 and RFC6648, it's easier to be more liberal in what we generate
      and not cause problems for users of these services.
      
      This change does not address other problems noted in that ticket, such
      as SES claiming that a message/rfc822 cannot be inlined.
      4f34a6f3
  5. 24 Aug, 2013 1 commit
  6. 23 Aug, 2013 1 commit
  7. 03 Jul, 2013 2 commits
    • Thomas Sibley's avatar
    • sunnavy's avatar
      refactor SetMIMEEncoding functionality( see also #17680 ) · bd42e5a2
      sunnavy authored
      incoming mail:
      
      1) find encoding
      2) if found then try to convert to utf-8 in croak mode, return if success
      3) guess encoding
      4) if guessed differently then try to convert to utf-8 in croak mode, return if success
      5) mark part as application/octet-stream instead of falling back to any encoding
      
      outgoing mail:
      
      1) find encoding
      2) if didn't find then do nothing, send as is, let MUA deal with it
      3) if found then try to convert it to outgoing encoding in croak mode, return if success
      4) do nothing otherwise, keep original encoding
      bd42e5a2
  8. 27 Jun, 2013 1 commit
    • Thomas Sibley's avatar
      Default to the "attachment" disposition when none is specified · 2ce35914
      Thomas Sibley authored
      Attachments received to RT via email usually have a Content-disposition
      header, which we continue to respect.
      
      However, attachments uploaded via the web will not and rely on the
      default here in AddAttachment().  Previously we relied on MIME::Entity's
      default, but it is not appropriate to assume all files are inline-able.
      
      Indeed, common sense dictates that attachments be treated as
      attachments, which is in line with user expectations.  Lenient MUAs
      which don't always respect the inline disposition have kept us from
      noticing when RT sends mail demanding PDFs, sound files, and other
      binaries are inlined.
      2ce35914
  9. 24 May, 2013 1 commit
  10. 09 May, 2013 1 commit
  11. 10 Apr, 2013 1 commit
  12. 07 Jan, 2013 1 commit
  13. 27 Dec, 2012 1 commit
    • Alex Vandiver's avatar
      Remove SMTP delivery option, which can drop email · 2300617f
      Alex Vandiver authored
      The 'smtp' delivery mode, despite being supported, was a poor choice of
      delivery, as in the case where the chosen sever failed to respond, it
      would silently drop the message.  Remove the option, so that this does
      not bite users.
      2300617f
  14. 25 Oct, 2012 7 commits
  15. 08 Oct, 2012 2 commits
  16. 01 Sep, 2012 2 commits
  17. 15 May, 2012 1 commit
    • Alex Vandiver's avatar
      AddAttachments must use $RT::SystemUser when searching for attachments to use · 14b8b16c
      Alex Vandiver authored
      c29107c6 changed AddAttachments to use the transaction's current user to
      search for which attachments to add to the outgoing mail.
      Unfortunately, this ignored the common case where the transaction's
      current user is an unprivileged user who does not have rights to see
      their own attachment.  This manifested itself as AdminCc emails not
      having attachments which were included with the original mail that
      triggered them, despite RT-Attach-Message being set.
      
      Revert the CurrentUser on the Attachments search to $RT::SystemUser, as
      it was pre- c29107c6.  This does not re-open the vulnerability, as
      (unlike the AddTicket functionality) the transaction creator can only
      cause attachments on their own transaction to be distributed.  While one
      route to fix this would be to modify RT::Attachments->Next to allow
      creators to always see their own attachments, such a change might have
      broader-reaching implications.
      
      Conflicts:
      
      	lib/RT/Action/SendEmail.pm
      14b8b16c
  18. 14 May, 2012 1 commit
    • Alex Vandiver's avatar
      AddAttachments must use RT->SystemUser when searching for attachments to use · ddb3ab99
      Alex Vandiver authored
      c29107c6 changed AddAttachments to use the transaction's current user to
      search for which attachments to add to the outgoing mail.
      Unfortunately, this ignored the common case where the transaction's
      current user is an unprivileged user who does not have rights to see
      their own attachment.  This manifested itself as AdminCc emails not
      having attachments which were included with the original mail that
      triggered them, despite RT-Attach-Message being set.
      
      Revert the CurrentUser on the Attachments search to RT->SystemUser, as
      it was pre- c29107c6.  This does not re-open the vulnerability, as
      (unlike the AddTicket functionality) the transaction creator can only
      cause attachments on their own transaction to be distributed.  While one
      route to fix this would be to modify RT::Attachments->Next to allow
      creators to always see their own attachments, such a change might have
      broader-reaching implications.
      ddb3ab99
  19. 30 Mar, 2012 1 commit
    • Thomas Sibley's avatar
      Iterate attachments as the creator of the current transaction when sending mail · 5170d905
      Thomas Sibley authored
      Check CurrentUserCanSee before trying to add an attachment since it
      could otherwise end up empty now that we have the correct current user.
      
      Additionally, simply check RT::Transaction's CurrentUserCanSee when
      iterating inside an RT::Attachments object rather than maintaining a
      different but similar conditional tree.  CurrentUserCanSee correctly
      access checks transaction types like EmailRecord, for example.
      
      This resolves part of CVE-2011-2084.
      5170d905
  20. 13 Jan, 2012 1 commit
    • Thomas Sibley's avatar
      Preserve the dispositions of attachments when redistributing mail · 8cbcb67b
      Thomas Sibley authored
      Use the original Content-Disposition if there is one, otherwise let
      MIME::Entity default to inline.
      
      MIME::Entity->build (used by ->attach) defaults to an inline disposition
      if none is specified.  This had the affect of mutating all incoming
      parts with an attachment disposition into inline content when RT
      redistributed it to watchers.
      8cbcb67b
  21. 03 Jan, 2012 1 commit
  22. 16 Nov, 2011 1 commit
    • Thomas Sibley's avatar
      Iterate attachments as the creator of the current transaction when sending mail · c29107c6
      Thomas Sibley authored
      Check CurrentUserCanSee before trying to add an attachment since it
      could otherwise end up empty now that we have the correct current user.
      
      Additionally, simply check RT::Transaction's CurrentUserCanSee when
      iterating inside an RT::Attachments object rather than maintaining a
      different but similar conditional tree.  CurrentUserCanSee correctly
      access checks transaction types like EmailRecord, for example.
      
      This resolves part of CVE-2011-2084.
      c29107c6
  23. 29 Sep, 2011 2 commits
  24. 20 May, 2011 2 commits
  25. 18 Mar, 2011 1 commit
    • Kevin Falcone's avatar
      Previously, we killed the RowsPerPage() call to avoid harming the cache · 81cb90b2
      Kevin Falcone authored
      Rather than trading performance to avoid the landmine of the
      Transaction->Attachments cache, we'll just built our own single element
      attachment collection.
      
      Calling RowsPerPage on the value returned from Transaction->Attachments
      means that any future code that calls Attachments on this Transaction
      object will only ever return a single attachment.  This fails
      predictably when Transaction->ContentObj wants to iterate through all
      your attachments but only gets 1.
      (cherry picked from commit 277ee932)
      81cb90b2