Prevent crafted DOS attack. #2501
Conversation
JimBobSquarePants
requested review from
tocsoft,
antonfirsov,
dlemstra and
brianpopow
| @@ -28,7 +28,7 @@ public static void SkipWhitespaceAndComments(this BufferedReadStream stream) | |||
| { | |||
| innerValue = stream.ReadByte(); | |||
| } | |||
| while (innerValue != 0x0a); | |||
| while (innerValue is not 0x0a and not -0x1); | |||
There was a problem hiding this comment.
Should a similar check be added also in line 52 ?
There was a problem hiding this comment.
int current = stream.ReadByte() - 0x30;
if ((uint)current > 9)
{
break;
}In case of EOF current == -1 -0x30 == -0x31. (uint)-0x31 == SomeHighNum > 9.
Nevertheless, a test case might be nice, these infinite loops are dangerous. 💀
There was a problem hiding this comment.
Yeah, that's why I left it. We actually enter that loop in ReadDecimal with this test, but I'll double check before merging.
There was a problem hiding this comment.
We cover this code with the test so I'm happy. I'll open another PR after this one to backport the fix to 2.X
Done #2509 I was hoping to get the gif fixes in asap but they'll have to wait if I cannot get hem reviewed. |
Prerequisites
Description
It's possible to cause the library to hang with a crafted byte array targetting the PBM decoder. This PR fixes that vulnarability. All other formats correctly test for the end of the stream.