MimeKit icon indicating copy to clipboard operation
MimeKit copied to clipboard

[WIP] Improve MimeParser safety

Open iamcarbon opened this issue 4 years ago • 3 comments

Work toward the spanification of MimeParser. Posting for early feedback.

iamcarbon avatar Feb 08 '22 00:02 iamcarbon

I think this is all unnecessary, but I will inspect closer after work to verify.

I know it seems like ABR's are easily possible, but I'm fairly (90%?) certain they aren't.

jstedfast avatar Feb 08 '22 17:02 jstedfast

There are 5 uses of IsMboxMarker:

  1. Line #778: This is safe because we check markerLength >= 5 and allowMunged is false.
  2. Line #928: We also check length >= 5 and allowMunged is false.
  3. Line #939: we check length < 5 (if < 5 we don't call IsMboxMarker), but we do use allowMunged = true so we theoretically could compare up to 6 input bytes. I'll investigate this a bit more later, but I'm pretty sure even this one is safe (even if only because MimeParser.input has 4 bytes of unused padding at the end).
  4. Line #1194: We check length >= 5 and allowMunged = false, so this isn't an issue.
  5. Line #1211: This is in the IsBoundary() method which only gets called when we've located the end of the line, so there will be a '\n' byte which will cause the "From " check to fail and prevent any possibility of an ABR (even discounting the fact that the input buffer has padding).

jstedfast avatar Feb 08 '22 17:02 jstedfast

FWIW, the ConvertToCString() method is a helper method that I sometimes use in the debugger (as invoked in the Immediate window or the Watch window), it's not used by code.

jstedfast avatar Feb 08 '22 17:02 jstedfast