Download raw body.
isakmpd: avoid undefined function pointer casts
On Wed, Jul 16, 2025 at 09:46:22PM +0200, Theo Buehler wrote:
> This is a bit of a stupid diff with lots of ugly boilerplate, but I
> don't see a better way. The problem is the second sentence of
> C99 6.3.2.3, 8:
>
> A pointer to a function of one type may be converted to a pointer to a
> function of another type and back again; the result shall compare equal
> to the original pointer. If a converted pointer is used to call a
> function whose type is not compatible with the pointed-to type, the
> behavior is undefined.
>
> The notion of compatibility for the functions here boils down to
> compatible return type and compatible argument list, which in particular
> means the same number of arguments. See 6.7.5.3, 15 for the gory details.
> There's also a version understandable to a human being lacking the brain
> damage necessary for successfully deciphering the C99 standard's language:
> https://en.cppreference.com/w/c/language/type.html
>
> So calling 'void CKSUM_Final(CKSUM_CTX *);' through the final member of
> struct hash_function is clearly UB since the numbers of parameters don't
> match. If that was all, the fix would be trivial. But the same goes for
> void MD5Final(MD5_CTX *, char *); because MD5_CTX * and void * aren't
> compatible in this sense: by definition, two pointer types are compatible
> if they point to compatible types. Or, if you prefer: void * has looser
> alignment requirements than MD5_CTX *, so they aren't compatible.
>
> The below avoids this by adding wrappers that have the correct types
> and that then propagate their parameters in a defined way. As I said,
> it's stupid and ugly and the one upside apart from appeasing the
> language lawyers I can see is that we no longer have function pointer
> casts.
>
> PS: there is similar code in isakmpd/hash.c.
Here's the corresponding change for isakmpd:
Index: hash.c
===================================================================
RCS file: /cvs/src/sbin/isakmpd/hash.c,v
diff -u -p -r1.25 hash.c
--- hash.c 21 Nov 2024 10:07:30 -0000 1.25
+++ hash.c 17 Jul 2025 06:31:42 -0000
@@ -43,7 +43,7 @@ void hmac_init(struct hash *, unsigned c
void hmac_final(unsigned char *, struct hash *);
/* Temporary hash contexts. */
-static union {
+static union ANY_CTX {
MD5_CTX md5ctx;
SHA1_CTX sha1ctx;
SHA2_CTX sha2ctx;
@@ -54,45 +54,135 @@ static unsigned char digest[HASH_MAX];
/* Encapsulation of hash functions. */
+static void
+md5_init(union ANY_CTX *ctx)
+{
+ MD5Init(&ctx->md5ctx);
+}
+
+static void
+md5_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
+{
+ MD5Update(&ctx->md5ctx, data, len);
+}
+
+static void
+md5_final(unsigned char *digest, union ANY_CTX *ctx)
+{
+ MD5Final(digest, &ctx->md5ctx);
+}
+
+static void
+sha1_init(union ANY_CTX *ctx)
+{
+ SHA1Init(&ctx->sha1ctx);
+}
+
+static void
+sha1_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
+{
+ SHA1Update(&ctx->sha1ctx, data, len);
+}
+
+static void
+sha1_final(unsigned char *digest, union ANY_CTX *ctx)
+{
+ SHA1Final(digest, &ctx->sha1ctx);
+}
+
+static void
+sha256_init(union ANY_CTX *ctx)
+{
+ SHA256Init(&ctx->sha2ctx);
+}
+
+static void
+sha256_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
+{
+ SHA256Update(&ctx->sha2ctx, data, len);
+}
+
+static void
+sha256_final(unsigned char *digest, union ANY_CTX *ctx)
+{
+ SHA256Final(digest, &ctx->sha2ctx);
+}
+
+static void
+sha384_init(union ANY_CTX *ctx)
+{
+ SHA384Init(&ctx->sha2ctx);
+}
+
+static void
+sha384_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
+{
+ SHA384Update(&ctx->sha2ctx, data, len);
+}
+
+static void
+sha384_final(unsigned char *digest, union ANY_CTX *ctx)
+{
+ SHA384Final(digest, &ctx->sha2ctx);
+}
+
+static void
+sha512_init(union ANY_CTX *ctx)
+{
+ SHA512Init(&ctx->sha2ctx);
+}
+
+static void
+sha512_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
+{
+ SHA512Update(&ctx->sha2ctx, data, len);
+}
+
+static void
+sha512_final(unsigned char *digest, union ANY_CTX *ctx)
+{
+ SHA512Final(digest, &ctx->sha2ctx);
+}
+
static struct hash hashes[] = {
{
HASH_MD5, 5, MD5_SIZE, MD5_BLOCK_LENGTH, (void *)&Ctx.md5ctx, digest,
sizeof(MD5_CTX), (void *)&Ctx2.md5ctx,
- (void (*)(void *))MD5Init,
- (void (*)(void *, unsigned char *, unsigned int))MD5Update,
- (void (*)(unsigned char *, void *))MD5Final,
+ md5_init,
+ md5_update,
+ md5_final,
hmac_init,
hmac_final
}, {
HASH_SHA1, 6, SHA1_SIZE, SHA1_BLOCK_LENGTH, (void *)&Ctx.sha1ctx,
digest, sizeof(SHA1_CTX), (void *)&Ctx2.sha1ctx,
- (void (*)(void *))SHA1Init,
- (void (*)(void *, unsigned char *, unsigned int))SHA1Update,
- (void (*)(unsigned char *, void *))SHA1Final,
+ sha1_init,
+ sha1_update,
+ sha1_final,
hmac_init,
hmac_final
}, {
HASH_SHA2_256, 7, SHA2_256_SIZE, SHA256_BLOCK_LENGTH,
(void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
- (void (*)(void *))SHA256Init,
- (void (*)(void *, unsigned char *, unsigned int))SHA256Update,
- (void (*)(u_int8_t *, void *))SHA256Final,
+ sha256_init,
+ sha256_update,
+ sha256_final,
hmac_init,
hmac_final
}, {
HASH_SHA2_384, 8, SHA2_384_SIZE, SHA384_BLOCK_LENGTH,
(void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
- (void (*)(void *))SHA384Init,
- (void (*)(void *, unsigned char *, unsigned int))SHA384Update,
- (void (*)(u_int8_t *, void *))SHA384Final,
+ sha384_init,
+ sha384_update,
+ sha384_final,
hmac_init,
hmac_final
}, {
HASH_SHA2_512, 9, SHA2_512_SIZE, SHA512_BLOCK_LENGTH,
(void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
- (void (*)(void *))SHA512Init,
- (void (*)(void *, unsigned char *, unsigned int))SHA512Update,
- (void (*)(u_int8_t *, void *))SHA512Final,
+ sha512_init,
+ sha512_update,
+ sha512_final,
hmac_init,
hmac_final
}
Index: hash.h
===================================================================
RCS file: /cvs/src/sbin/isakmpd/hash.h,v
diff -u -p -r1.9 hash.h
--- hash.h 21 Nov 2024 10:07:30 -0000 1.9
+++ hash.h 17 Jul 2025 06:34:50 -0000
@@ -49,6 +49,8 @@ enum hashes {
HASH_SHA2_512
};
+union ANY_CTX;
+
struct hash {
enum hashes type;
int id; /* ISAKMP/Oakley ID */
@@ -58,9 +60,9 @@ struct hash {
unsigned char *digest; /* Pointer to a digest */
int ctxsize;
void *ctx2; /* Pointer to a 2nd context, for HMAC octx */
- void (*Init) (void *);
- void (*Update) (void *, unsigned char *, unsigned int);
- void (*Final) (unsigned char *, void *);
+ void (*Init) (union ANY_CTX *);
+ void (*Update) (union ANY_CTX *, const unsigned char *, size_t);
+ void (*Final) (unsigned char *, union ANY_CTX *);
void (*HMACInit) (struct hash *, unsigned char *, unsigned int);
void (*HMACFinal) (unsigned char *, struct hash *);
};
isakmpd: avoid undefined function pointer casts