search

emersion / go-smtp

📤 An SMTP client & server library written in Go
MIT License
1.72k stars 216 forks source link

Remove DotLF to EOFState case #244

Closed blmayer closed 9 months ago

blmayer commented 9 months ago

Hi, this should be enough for closing #243. However, since this is a small code change, I did not test anything.

emersion commented 9 months ago

I've written a small test for this here: https://github.com/emersion/go-smtp/tree/smtp-smuggling-test

Could you cherry-pick that commit and make sure that it passes?

blmayer commented 9 months ago

Yes.

blmayer commented 9 months ago

The test failed due to a timeout. I think scanner.Scan() doesn't return, as it is expecting the correct end. I added a smaller timeout to that test, and removed the check for an incorrect end, since the server doesn't error out: it is correctly expecting more data.

See if you agree with this.

emersion commented 9 months ago

Ah right, of course. I've pushed another variant of the test which shouldn't cause a timeout.

blmayer commented 9 months ago

Cool, much better, I had to adjust the check's text.

emersion commented 9 months ago

For reference, excerpt from the RFC:

The custom of accepting lines ending only in <LF>, as a concession to
non-conforming behavior on the part of some UNIX systems, has proven
to cause more interoperability problems than it solves, and SMTP
server systems MUST NOT do this, even in the name of improved
robustness.  In particular, the sequence "<LF>.<LF>" (bare line
feeds, without carriage returns) MUST NOT be treated as equivalent to
<CRLF>.<CRLF> as the end of mail data indication.
emersion commented 9 months ago

Thanks for the fix!