minor cleanups

use correct da_is_length_field() macro
"too long" data isn't an assert.  It's a "can't encode" error

diff --git a/src/protocols/dhcpv6/decode.c b/src/protocols/dhcpv6/decode.c
index ac604a3f1c93a9cf72666287e7ef7280e863e7f9..7771bd739015446ecddfdba6a3bfc00bb95b1bca 100644 (file)
--- a/src/protocols/dhcpv6/decode.c
+++ b/src/protocols/dhcpv6/decode.c
@@ -296,6 +296,7 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out,
                         *      decode the last bit as raw data.
                         */
                        if ((size_t) (end - p) < element_len) {
+                       raw:
                                slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx);
                                if (slen < 0) return slen;
                                break;
@@ -319,22 +320,20 @@ static ssize_t decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      If the data is variable length i.e. strings or octets
         *      there is a length field before each element.
         *
+        *      Note that we don't bother checking if the data type is
+        *      string or octets.  There will only be issues if
+        *      someone edited the dictionaries and broke them.
+        *
         *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
         *   |       text-len                |        String                 |
         *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
         */
        while (p < end) {
-               if ((end - p) < 2) {
-               raw:
-                       slen = decode_raw(ctx, out, parent, p, end - p , decode_ctx);
-                       if (slen < 0) return slen;
-                       break;
-               }
+               if ((end - p) < 2) goto raw;
 
                element_len = fr_net_to_uint16(p);
-               if ((p + 2 + element_len) > end) {
-                       goto raw;
-               }
+
+               if ((size_t) (end - p) < (((size_t) element_len) + 2)) goto raw;
 
                p += 2;
                slen = decode_value(ctx, out, parent, p, element_len, decode_ctx);

diff --git a/src/protocols/dhcpv6/encode.c b/src/protocols/dhcpv6/encode.c
index a202d8101084ddb38dea40625ac0d65e9db33e32..eb552ec79f798683476425ab4c2ae3c6b2100ad4 100644 (file)
--- a/src/protocols/dhcpv6/encode.c
+++ b/src/protocols/dhcpv6/encode.c
@@ -441,14 +441,14 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff,
                 *   |       text-len                |        String                 |
                 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
                 */
-               if (!da->flags.length) {
+               if (da_is_length_field(da)) {
                        len_field = true;
                        FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint16_t));      /* Make room for the length field */
                }
 
                slen = encode_value(&element_dbuff, da_stack, depth, cursor, encode_ctx);
                if (slen < 0) return slen;
-               if (!fr_cond_assert(slen < UINT16_MAX)) return PAIR_ENCODE_FATAL_ERROR;
+               if (slen > UINT16_MAX) return PAIR_ENCODE_FATAL_ERROR;
 
                /*
                 *      Ensure we always create elements of the correct length.
@@ -467,10 +467,9 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff,
                /*
                 *      Populate the length field
                 */
-               if (len_field) fr_dbuff_in(&work_dbuff, (uint16_t) slen);
+               else if (len_field) fr_dbuff_in(&work_dbuff, (uint16_t) slen);
                fr_dbuff_set(&work_dbuff, &element_dbuff);
 
-
                vp = fr_dcursor_current(cursor);
                if (!vp || (vp->da != da)) break;               /* Stop if we have an attribute of a different type */
        }