Remove a redundant variable-length array

In the GSS-TSIG verification code there was an alarming
variable-length array whose size came off the network, from the
signature in the request. It turned out to be safe, because the caller
had previously checked that the signature had a reasonable size.
However, the safety checks are in the generic TSIG implementation, and
the risky VLA usage was in the GSS-specific code, and they are
separated by the DST indirection layer, so it wasn't immediately
obvious that the risky VLA was in fact safe.

In fact this risky VLA was completely unnecessary, because the GSS
signature can be verified in place without being copied to the stack,
like the message covered by the signature. The `REGION_TO_GBUFFER()`
macro backwardly assigns the region in its left argument to the GSS
buffer in its right argument; this is just a pointer and length
conversion, without copying any data. The `gss_verify_mic()` call uses
both message and signature GSS buffers in a read-only manner.

diff --git a/CHANGES b/CHANGES
index bbf5ebd97c8624e76e902103813bc685e89f115b..3aef12ec0e5623f5e6f9ee566e89102c4d6fcf39 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5834.  [cleanup]       C99 variable-length arrays are difficult to use safely,
+                       so avoid them except in test code. [GL #3201]
+
 5828.  [bug]           Replace single TCP write timer with per-TCP write
                        timers. [GL #3200]
 

diff --git a/lib/dns/gssapi_link.c b/lib/dns/gssapi_link.c
index 8b89067ea2bd64f146b564feeac92ed8f0742a45..966cc2c6cbad463310f682fcaba3e2a809cd2d03 100644 (file)
--- a/lib/dns/gssapi_link.c
+++ b/lib/dns/gssapi_link.c
@@ -177,11 +177,10 @@ gssapi_sign(dst_context_t *dctx, isc_buffer_t *sig) {
 static isc_result_t
 gssapi_verify(dst_context_t *dctx, const isc_region_t *sig) {
        dst_gssapi_signverifyctx_t *ctx = dctx->ctxdata.gssctx;
-       isc_region_t message, r;
+       isc_region_t message;
        gss_buffer_desc gmessage, gsig;
        OM_uint32 minor, gret;
        gss_ctx_id_t gssctx = dctx->key->keydata.gssctx;
-       unsigned char buf[sig->length];
        char err[1024];
 
        /*
@@ -190,11 +189,7 @@ gssapi_verify(dst_context_t *dctx, const isc_region_t *sig) {
         */
        isc_buffer_usedregion(ctx->buffer, &message);
        REGION_TO_GBUFFER(message, gmessage);
-
-       memmove(buf, sig->base, sig->length);
-       r.base = buf;
-       r.length = sig->length;
-       REGION_TO_GBUFFER(r, gsig);
+       REGION_TO_GBUFFER(*sig, gsig);
 
        /*
         * Verify the data.