Replace mblen() with bounds-checked versions.

[MasahikoSawada]

Malformed or corrupted strings could cause code iterating with pg_mblen() to overrun its buffer. This commit replaces calls to pg_mblen() and t_isspace() with bounds-checked versions, such as pg_mblen_with_len() and t_isspace_with_len().

For backward compatibility with legacy releases (PostgreSQL 13 and earlier), provide shims that map these new APIs to the traditional un-checked implementations.

Inspired by pg_trgm changes in commit 319e8a64419a.

[MasaoFujii]

@MasahikoSawada
Thanks for the patch!

I prepared a revised version based on this approach:
master...MasaoFujii:pg_bigm:fix-bounds-checked-mblen

The revised patch still uses the bounds-checked multibyte-length APIs, but routes all calls through pg_bigm-local wrappers. This avoids build failures on older supported PostgreSQL minor releases where the new APIs are unavailable, and also avoids exporting compatibility shim symbols with the same names as PostgreSQL core APIs.

It also limits pg_mblen_unbounded() to strings constructed by pg_bigm itself.

Thoughts?

[MasahikoSawada]

Thank you for the proposal!

The revised patch still uses the bounds-checked multibyte-length APIs, but routes all calls through pg_bigm-local wrappers. This avoids build failures on older supported PostgreSQL minor releases where the new APIs are unavailable, and also avoids exporting compatibility shim symbols with the same names as PostgreSQL core APIs.

Good point. I agree with this approach. One minor comment is:

#define BIGM_HAVE_BOUNDS_CHECKED_MBLEN \
	((PG_VERSION_NUM >= 140021 && PG_VERSION_NUM < 150000) || \
	 (PG_VERSION_NUM >= 150016 && PG_VERSION_NUM < 160000) || \
	 (PG_VERSION_NUM >= 160012 && PG_VERSION_NUM < 170000) || \
	 (PG_VERSION_NUM >= 170008 && PG_VERSION_NUM < 180000) || \
	 PG_VERSION_NUM >= 180002)

BIGM_HAVE_* reads like an autoconf feature flag, but it is defined unconditionally with a truth value in the body. So reader might want to use it like #ifdef BIGM_HAVE_BOUNDS_CHECKED_MBLEN but it's always true.

I'd suggest rewriting it to:

#if  (PG_VERSION_NUM >= 140021 && PG_VERSION_NUM < 150000) || \
	 (PG_VERSION_NUM >= 150016 && PG_VERSION_NUM < 160000) || \
	 (PG_VERSION_NUM >= 160012 && PG_VERSION_NUM < 170000) || \
	 (PG_VERSION_NUM >= 170008 && PG_VERSION_NUM < 180000) || \
	 PG_VERSION_NUM >= 180002
#define BIGM_HAVE_BOUNDS_CHECKED_MBLEN
#endif

Then use it like #ifndef BIGM_HAVE_BOUNDS_CHECKED_MBLEN.

[MasaoFujii]

@MasahikoSawada

BIGM_HAVE_* reads like an autoconf feature flag, but it is defined unconditionally with a truth value in the body. So reader might want to use it like #ifdef BIGM_HAVE_BOUNDS_CHECKED_MBLEN but it's always true.

I'd suggest rewriting it to:

Thanks for the review! You're right. I've updated the patch as suggested.
master...MasaoFujii:pg_bigm:fix-bounds-checked-mblen

If this looks good, I'll squash the two changes into one and commit it.

Regards,

[MasahikoSawada]

LGTM. Thanks!