improve smtpcode() input validation #190
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DerDakon
requested review from
leahneukirchen,
schmonz,
jlmuir,
mbhangui,
xenotrope and
josuah
schmonz
requested changes
Nov 26, 2020
smtpcode.c
Outdated
| err += get_digit(&ch); code = ch - '0'; | ||
| /* valid SMTP codes are >= 200 */ | ||
| if (ch < '2') | ||
| err = 1; |
There was a problem hiding this comment.
I expected err++ given how it's being used elsewhere
There was a problem hiding this comment.
In theory one could send so much crap that it will eventually overflow and pass as good, but why would one send 2^32 errors just to fool us to think the mail was accepted?
There was a problem hiding this comment.
This would happen if the input is closed during the loop, after 200 for instance for 200 EOF?
But the check on the content would prevent the loop to go till the end.
replacing for(;;) by while(err == 0) would cut my superstition.
There was a problem hiding this comment.
But that would break that this loop reads in all lines of a multiline reply.
DerDakon
force-pushed
the
Dakon-smtpcode-test
branch
2 times, most recently
from
e4cc593
to
d910d91
Compare
josuah
approved these changes
Nov 28, 2020
DerDakon
force-pushed
the
Dakon-smtpcode-test
branch
4 times, most recently
from
b120689
to
b187f97
Compare
leahneukirchen
approved these changes
Nov 15, 2022
mbhangui
approved these changes
Nov 15, 2022
…cter The server reply must be of the form (\d\d\d-[^\r\n]*\r\n)* \d\d\d [^\r\n]*\r\n Until now it was neither verified that the first 3 characters were digits, nor that the following character was really SPACE or MINUS. In case any of these restrictions violated return a high, invalid, SMTP code, that will cause all callers to return permanent error. While at it make sure noone can send an absurdly low number as status code, and return an error in that case, too.
Solaris does not seems to like the previous layout of the makefile. While the bug is still not identified, this version seems to avoid it.
DerDakon
force-pushed
the
Dakon-smtpcode-test
branch
from
b187f97
to
ea28e34
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One could basically pass anything in the first 3 characters of a line, and if it happened to have combined ASCII codes below 400 it would be taken as acceptance.