On Tue, Jan 05, 2021 at 12:21:09PM -0500, Bruce Momjian wrote: > Well, if the backend uses /common for hex like I suggested, and like we > do now, it has to match the function signatures of bytea and esc, see > struct pg_encoding. I don't see the point in changing those.
Not necessarily with some thin wrappers to adapt. And we only care about the source string for the escape format, not for hex. It seems to me that a huge point in redesigning the hex APIs is that we can make them more security-aware. We had one CVE in 2019 for SCRAM because of the base64 ones in src/common/ that got copied from encode.c. And I'd rather avoid taking any unnecessary risks here, particularly as this is going to be used for more security-related features. > The cfbot is all green for this patch now, so we are making progress. ;-) Ah, cool. > I think we are best leaving ecpglib alone, since it is a library, and > just have one other hex implementation in /common for all other cases. > > I found serious confusion in encoding.c: > > * function pointer prototype argument names didn't match function > prototypes > * used dlen for datalen, and data where src was used in real functions > * used len for src and dst len > * used dstlen for srclen It would be as well just a separate cleanup patch? > It was very confusing, and this attached patch fixes all of that. I > also added the pg_ prefix you suggrested. If we want to add dstlen to > all the functions, we have to do it for all types --- not sure it is > worth it, now that things are much clearer. Overflow protection is very important in my opinion here once we expose more those functions. Not touching ECPG at all and not making this refactoring pluggable into shared libs is fine by me at the end because we don't have a case for libpq yet, but I object to the lack of protection against overflows. -- Michael
signature.asc
Description: PGP signature
