Harden ECIES primitive: restore curve validation and reject short ciphertexts #1719
The application-level gate here looks right, but the root cause is still live in github.com/gonka-ai/cosmos-sdk: crypto/ecies diverges from the go-ethereum code it was copied from in two places — GenerateShared dropped the IsOnCurve check, and Decrypt's length gate is rLen+hLen+1 (98) instead of rLen+hLen+params.BlockSize (113). Anything in the 98–112 band reaches symDecrypt, where make([]byte, len(ct)-params.BlockSize) is a negative length and panics.
That means this PR fixes one caller, not the primitive. kr.Decrypt / ecies.Decrypt stays panic-on-attacker-input for every other consumer, now and future, and the new decryptKeyring recover is what stands between us and a process kill. Better to fix it upstream in the fork — restore the IsOnCurve check in GenerateShared and raise the Decrypt floor to rLen+hLen+params.BlockSize, matching geth — and keep this PR as defense-in-depth. Worth adding the 98-byte regression test on the fork side too, since ecies_short_ciphertext_test.go currently reimplements the fork's KDF/MAC internals from outside the module just to reach the panic.
I think it is good to fix primitive, while PR sounds and closes what it should
Originally posted by @a-kuprin in https://github.com/gonka-ai/gonka/issues/1687#issuecomment-5509311830
💬 Comments (1)
🔄 Auto-synced from Issue #1719 every hour.
Picked this up: gonka-ai/cosmos-sdk#20 restores both guards verbatim from go-ethereum (
IsOnCurveinGenerateShared,rLen + hLen + params.BlockSizefloor inDecrypt) and adds the fork-side regression tests, including the 98-byte case, which panics onrelease/v0.53.xtoday.One thing to plan for when the fork tag is bumped here:
TestDecryptKeyringShortMACValidReturnsErrorindecentralized-api/cosmosclientasserts on the"ecies decrypt panic"message. With the primitive fixed,Decryptreturnsecies: invalid messageinstead, so that assertion needs to becomerequire.Error. Happy to open that follow-up alongside thego.modbump.