Use checked arithmetic to prevent stack overflow in Pkcs12Kdf #68422
Conversation
When calculating the length of P and I, it's possible for this to result in an arithmetic overflow. This arithmetic overflow is then fed in to `stackalloc`, which treats the input as unsigned and causes a large stack allocation.
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsWhen calculating the length of P and I, it's possible for this to result in an arithmetic overflow. Closes #68419
|
|
could you add your repro as a test perhaps? |
I thought about it, but unfortunately it requires quite a large allocation. I've seen a number of test failures from tests that try to hit those conditions, so erred on the side of not testing it. If we have a test pattern that I can follow for ~2 GB allocations then I think it would make sense. |
|
There are a few non-const stackallocs in that class, isn't that bad practice by now and should be cleaned up? runtime/src/libraries/Common/src/System/Security/Cryptography/Pkcs12Kdf.cs Lines 140 to 142 in 52e93d9 (Besides having a stackalloc with 1024 byte size) |
We have a number of tests that do this and we made them stable by making them run alone (search for DisableParallelism) + limiting to 64 bit. I suppose one could count OutOfMemoryException as a pass, too. I don't have an opinion on whether it is worth it in this case though. |
|
I think we can get away without a test. |
When calculating the length of P and I, it's possible for this to result in an arithmetic overflow.
This arithmetic overflow is then fed in to
stackalloc, which treats the input as unsigned and causes a large stack allocation.Closes #68419