Re: pg_cryptohash_final possible out-of-bounds access (per Coverity)
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Ranier Vilela <ranier.vf@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2021-02-11T12:47:21Z
Lists: pgsql-hackers
Attachments
- pg_cryptohash_v2.patch (text/x-diff) patch v2
On Wed, Feb 10, 2021 at 09:14:46AM -0300, Ranier Vilela wrote: > It is necessary to correct the interfaces. To caller, inform the size of > the buffer it created. Well, Coverity likes nannyism, so each one of its reports is to take with a pinch of salt, so there is no point to change something that does not make sense just to please a static analyzer. The context of the code matters. Now, the patch you sent has no need to be that complicated, and it partially works while not actually solving at all the problem you are trying to solve (nothing done for MD5 or OpenSSL). Attached is an example of what I finish with while poking at this issue. There is IMO no point to touch the internals of SCRAM that all rely on the same digest lengths for the proof generation with SHA256. > I think it should be error-out, because the buffer can be malloc. I don't understand what you mean here, but cryptohash[_openssl].c should not issue an error directly, just return a status code that the caller can consume to generate an error. -- Michael
Commits
-
Add result size as argument of pg_cryptohash_final() for overflow checks
- b83dcf792869 14.0 landed