Skip to content

Commit

Permalink
net_smtp: Properly reject messages with long lines.
Browse files Browse the repository at this point in the history
RFC 5322 says that lines in a message cannot exceed
998 characters. We use a sufficiently sized buffer
to accomodate this requirement, but previously
bbs_readline would return the same value if the buffer
was exhausted as if read or poll had failed, and so
the SMTP server would just abruptly disconnect if a
line that was longer than the spec allowed was received.

Now, a different return value is used, allowing the
SMTP server to differentiate buffer exhaustion from
I/O failure, and we return a proper SMTP error code
before disconnecting.
  • Loading branch information
InterLinked1 committed Nov 24, 2023
1 parent 3b479c6 commit 7b38c23
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 5 deletions.
2 changes: 1 addition & 1 deletion bbs/readline.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ ssize_t bbs_readline(int fd, struct readline_data *restrict rldata, const char *
#endif
if (rldata->left - 1 < 2) {
bbs_warning("Buffer (size %lu) has been exhausted\n", rldata->len); /* The using application needs to allocate a larger buffer */
return -1;
return -3;
}
res = bbs_poll_read(fd, timeout, rldata->pos, (size_t) rldata->left - 1); /* Subtract 1 for NUL */
if (res <= 0) {
Expand Down
11 changes: 7 additions & 4 deletions include/readline.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,13 @@ int readline_bytes_available(struct readline_data *restrict rldata, int process)
* \param rldata Previously initialized using bbs_readline_init
* \param delim A delimiter (can be multiple characters). CR LF is typical for most network applications.
* \param timeout Timeout in ms for any call to poll()
* \retval -2 on failure, -1 if read or poll returns 0, and number of bytes read in first input chunk otherwise, not including the delimiter.
* A return value of 0 means that only the delimiter was read.
* If a positive value is returned, the caller can read the input in the buffer passed to bbs_readline_init. It will be null terminated after the first input chunk,
* not including the delimiter.
* \retval -3 on buffer exhaustion
* \retval -2 if read or poll returned -1
* \retval -1 if read or poll returned 0
* \retval 0 Only the delimiter was read.
* \return Number of bytes read in first input chunk otherwise, not including the delimiter.
* Caller can read input in the buffer passed to bbs_readline_init.
* It will be null terminated after the first input chunk, not including the delimiter.
* \note The actual number of bytes read may be greater than the number of bytes returned. These bytes will be returned in subsequent calls to this function.
*/
ssize_t bbs_readline(int fd, struct readline_data *restrict rldata, const char *restrict delim, int timeout);
Expand Down
11 changes: 11 additions & 0 deletions nets/net_smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -2364,6 +2364,17 @@ static int handle_data(struct smtp_session *smtp, char *s, struct readline_data
if (res < 0) {
bbs_delete_file(template);
fclose(fp);
if (res == -3) {
/* Buffer was exhausted.
* RFC 5322 2.1.1 says lines MUST be no longer than 998 characters.
* This is the sending mail user agent's resonsibility, not the responsibility of any MTA.
* Some user agents don't conform to this requirement, and some MTAs will happily relay them anyways.
* In practice, most messages that violate this seem to be spam anyways,
* so we have no reason to tolerate noncompliant messages.
* However, we need to reject it properly with the right error message before disconnecting. */
smtp_reply_nostatus(smtp, 550, "Maximum line length exceeded");
smtp->failures += 3; /* Semantically, this is a bad client. However, we're just going to disconnect now anyways, so this doesn't really matter. */
}
return -1;
}
s = rldata->buf;
Expand Down
23 changes: 23 additions & 0 deletions tests/test_smtp_mta.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,29 @@ static int run(void)
SWRITE(clientfd, "." ENDL); /* EOM */
CLIENT_EXPECT(clientfd, "554"); /* Mail loop detected */

#define CHAR_31 "abc def ghi jkl mno prs tuv wxy"
#define CHAR_248 CHAR_31 CHAR_31 CHAR_31 CHAR_31 CHAR_31 CHAR_31 CHAR_31 CHAR_31
#define CHAR_992 CHAR_248 CHAR_248 CHAR_248 CHAR_248

/* Ensure messages with lines over 1,000 characters are rejected */
SWRITE(clientfd, "RSET" ENDL);
CLIENT_EXPECT(clientfd, "250");
SWRITE(clientfd, "EHLO " TEST_EXTERNAL_DOMAIN ENDL);
CLIENT_EXPECT_EVENTUALLY(clientfd, "250 ");
SWRITE(clientfd, "MAIL FROM:<" TEST_EMAIL_EXTERNAL ">\r\n");
CLIENT_EXPECT(clientfd, "250");
SWRITE(clientfd, "RCPT TO:<" TEST_EMAIL ">\r\n");
CLIENT_EXPECT(clientfd, "250");
SWRITE(clientfd, "DATA\r\n");
CLIENT_EXPECT(clientfd, "354");
SWRITE(clientfd, "Date: Thu, 21 May 1998 05:33:29 -0700" ENDL);
SWRITE(clientfd, ENDL);
SWRITE(clientfd, "Test" ENDL);
SWRITE(clientfd, CHAR_992 ENDL); /* This is okay */
SWRITE(clientfd, CHAR_992 CHAR_31 ENDL); /* This is not okay */
SWRITE(clientfd, "." ENDL); /* EOM */
CLIENT_EXPECT(clientfd, "550"); /* Mail loop detected */

/* Test pregreeting */
close(clientfd);
clientfd = test_make_socket(25);
Expand Down

0 comments on commit 7b38c23

Please sign in to comment.