Make use of the fr_nbo_to*() functions where possible (#4691)

It appears to appease coverity complaints about tainted data.

diff --git a/src/lib/eap/chbind.c b/src/lib/eap/chbind.c
index 5d4798017f2c964d3710dfb18cf6772a3d6eca6d..01b893fe47c8972a108cd43522054d4a8c1077bd 100644 (file)
--- a/src/lib/eap/chbind.c
+++ b/src/lib/eap/chbind.c
@@ -149,7 +149,7 @@ static size_t chbind_get_data(chbind_packet_t const *packet,
                 */
                if ((end - ptr) < 4) return 0;
 
-               length = (ptr[0] << 8) | ptr[1];
+               length = fr_nbo_to_uint16(ptr);
                if (length == 0) return 0;
 
                if ((ptr + length + 3) > end) return 0;

diff --git a/src/lib/eap_aka_sim/decode.c b/src/lib/eap_aka_sim/decode.c
index 44d1df5b17e196737fb786409baf8955ab6f87b5..ee29bad479491365a1ba3e558ae26d37fc3a17d5 100644 (file)
--- a/src/lib/eap_aka_sim/decode.c
+++ b/src/lib/eap_aka_sim/decode.c
@@ -317,7 +317,7 @@ static ssize_t sim_decode_array(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      length.
         */
        if (!parent->flags.length || (parent->flags.length % 4)) {
-               actual_len = (p[0] << 8) | p[1];
+               actual_len = fr_nbo_to_uint16(p);
                if (actual_len > (attr_len - 2)) {
                        fr_strerror_printf("%s: Actual length field value (%hu) > attribute value length (%zu)",
                                           __FUNCTION__, actual_len, attr_len - 2);
@@ -585,7 +585,7 @@ static ssize_t sim_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_di
 
                if (attr_len < 2) goto raw;     /* Need at least two bytes for the length field */
 
-               res_len = (p[0] << 8) | p[1];
+               res_len = fr_nbo_to_uint16(p);
                if (res_len % 8) {
                        fr_strerror_printf("%s: RES Length (%hu) is not a multiple of 8",
                                           __FUNCTION__, res_len);
@@ -737,7 +737,7 @@ static ssize_t sim_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_di
         */
        case FR_TYPE_STRING:
        {
-               uint16_t actual_len = (p[0] << 8) | p[1];
+               uint16_t actual_len = fr_nbo_to_uint16(p);
 
                if (actual_len > (attr_len - 2)) {
                        fr_strerror_printf("%s: Actual length field value (%hu) > attribute value length (%zu)",
@@ -754,7 +754,7 @@ static ssize_t sim_decode_pair_value(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_di
                 *      Variable length octets buffer
                 */
                if (!parent->flags.length) {
-                       uint16_t actual_len = (p[0] << 8) | p[1];
+                       uint16_t actual_len = fr_nbo_to_uint16(p);
 
                        if (actual_len > (attr_len - prefix)) {
                                fr_strerror_printf("%s: Actual length field value (%hu) > attribute value length (%zu)",

diff --git a/src/lib/tls/session.c b/src/lib/tls/session.c
index 0921be31893c9f79278472860e6bd7bf75cc78a1..05207ad289d38c8594ed8a0775e0e2414a799c35 100644 (file)
--- a/src/lib/tls/session.c
+++ b/src/lib/tls/session.c
@@ -818,7 +818,7 @@ void fr_tls_session_msg_cb(int write_p, int msg_version, int content_type,
                if ((len >= 3) && (p[0] == 1)) {
                        size_t payload_len;
 
-                       payload_len = (p[1] << 8) | p[2];
+                       payload_len = fr_nbo_to_uint16(p + 1);
                        if ((payload_len + 3) > len) {
                                tls_session->invalid = true;
                                ROPTIONAL(REDEBUG, ERROR, "OpenSSL Heartbeat attack detected.  Closing connection");

diff --git a/src/lib/util/inet.c b/src/lib/util/inet.c
index 7f5c235768f15e4c611284e4c8df005cae3465c8..51494062ec6cb3a9a75337ea450581d33ebe8f33 100644 (file)
--- a/src/lib/util/inet.c
+++ b/src/lib/util/inet.c
@@ -1085,8 +1085,8 @@ char *fr_inet_ntop_prefix(char out[static FR_IPADDR_PREFIX_STRLEN], size_t outle
 char *fr_inet_ifid_ntop(char *out, size_t outlen, uint8_t const *ifid)
 {
        snprintf(out, outlen, "%x:%x:%x:%x",
-                (ifid[0] << 8) + ifid[1], (ifid[2] << 8) + ifid[3],
-                (ifid[4] << 8) + ifid[5], (ifid[6] << 8) + ifid[7]);
+                fr_nbo_to_uint16(ifid),     fr_nbo_to_uint16(ifid + 2),
+                fr_nbo_to_uint16(ifid + 4), fr_nbo_to_uint16(ifid + 6));
        return out;
 }
 

diff --git a/src/lib/util/missing.c b/src/lib/util/missing.c
index 9c5c41673700574630ac56294173823d882a070d..10c2aa7b44a823656bdc4115fdf425b84fb18265 100644 (file)
--- a/src/lib/util/missing.c
+++ b/src/lib/util/missing.c
@@ -463,14 +463,14 @@ char const *inet_ntop(int af, void const *src, char *dst, size_t cnt)
                if (cnt <= INET6_ADDRSTRLEN) return NULL;
 
                snprintf(dst, cnt, "%x:%x:%x:%x:%x:%x:%x:%x",
-                        (ipaddr->s6_addr[0] << 8) | ipaddr->s6_addr[1],
-                        (ipaddr->s6_addr[2] << 8) | ipaddr->s6_addr[3],
-                        (ipaddr->s6_addr[4] << 8) | ipaddr->s6_addr[5],
-                        (ipaddr->s6_addr[6] << 8) | ipaddr->s6_addr[7],
-                        (ipaddr->s6_addr[8] << 8) | ipaddr->s6_addr[9],
-                        (ipaddr->s6_addr[10] << 8) | ipaddr->s6_addr[11],
-                        (ipaddr->s6_addr[12] << 8) | ipaddr->s6_addr[13],
-                        (ipaddr->s6_addr[14] << 8) | ipaddr->s6_addr[15]);
+                        fr_nbo_to_uint16(ipaddr->a6_addr),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 2),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 4),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 6),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 8),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 10),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 12),
+                        fr_nbo_to_uint16(ipaddr->a6_addr + 14));
                return dst;
        }
 

diff --git a/src/listen/arp/proto_arp.c b/src/listen/arp/proto_arp.c
index 24d246fe659a55d5285634462bb299f706ed7f9d..a2a991a239f6ff765f0cb34de28299085f8eba3f 100644 (file)
--- a/src/listen/arp/proto_arp.c
+++ b/src/listen/arp/proto_arp.c
@@ -79,7 +79,7 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t *
        }
 
        arp = (fr_arp_packet_t const *) data;
-       request->packet->code = (arp->op[0] << 8) | arp->op[1];
+       request->packet->code = fr_nbo_to_uint16(arp->op);
        fr_assert(request->packet->code < FR_ARP_CODE_MAX);
 
        request->packet->data = talloc_memdup(request->packet, data, data_len);

diff --git a/src/listen/dhcpv4/proto_dhcpv4.c b/src/listen/dhcpv4/proto_dhcpv4.c
index 8146e64caa8e38276036f3f3d6c89a042fcf48dd..6b6e569ca61bd741ca8132520468617a145ef445 100644 (file)
--- a/src/listen/dhcpv4/proto_dhcpv4.c
+++ b/src/listen/dhcpv4/proto_dhcpv4.c
@@ -212,7 +212,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d
        /*
         *      Hacks for now until we have a lower-level decode routine.
         */
-       request->packet->id = (data[4] << 24) | (data[5] << 16) | (data[6] << 8) | data[7];
+       request->packet->id = fr_nbo_to_uint32(data + 4);
        request->reply->id = request->packet->id;
        memcpy(request->packet->vector, data + 4, sizeof(request->packet->vector));
 

diff --git a/src/listen/dhcpv6/proto_dhcpv6_udp.c b/src/listen/dhcpv6/proto_dhcpv6_udp.c
index 50808ef32e8dc5d10f4e7e6f28d21d2b1d671dd5..d93420a8c3c6bb8f884ff833132f5b02d6e140ec 100644 (file)
--- a/src/listen/dhcpv6/proto_dhcpv6_udp.c
+++ b/src/listen/dhcpv6/proto_dhcpv6_udp.c
@@ -195,7 +195,7 @@ static ssize_t mod_read(fr_listen_t *li, void **packet_ctx, fr_time_t *recv_time
         *      proto_dhcpv6 sets the priority
         */
 
-       xid = (packet->transaction_id[0] << 16) | (packet->transaction_id[1] << 8) | packet->transaction_id[2];
+       xid = fr_nbo_to_uint24(packet->transaction_id);
 
        /*
         *      Print out what we received.
@@ -398,7 +398,7 @@ static void *mod_track_create(UNUSED void const *instance, UNUSED void *thread_i
                option = fr_dhcpv6_option_find(packet + 2 + 32, packet + packet_len, attr_relay_message->attr);
                if (!option) return NULL;
 
-               option_len = (option[2] << 8) | option[3];
+               option_len = fr_nbo_to_uint16(option + 2);
 
                packet = option + 4; /* skip option header */
                packet_len = option_len;
@@ -412,7 +412,7 @@ static void *mod_track_create(UNUSED void const *instance, UNUSED void *thread_i
        option = fr_dhcpv6_option_find(packet + 4, packet + packet_len, attr_client_id->attr);
        if (!option) return NULL;
 
-       option_len = (option[2] << 8) | option[3];
+       option_len = fr_nbo_to_uint16(option + 2);
 
        t = (proto_dhcpv6_track_t *) talloc_zero_array(track, uint8_t, t_size + option_len);
        if (!t) return NULL;

diff --git a/src/listen/dns/proto_dns.c b/src/listen/dns/proto_dns.c
index 5d3621c780ccc2ca2f85f40ecc47a2ca7c892cb3..edce4b2c89be24f21d389ee36d35f690cb02457e 100644 (file)
--- a/src/listen/dns/proto_dns.c
+++ b/src/listen/dns/proto_dns.c
@@ -195,7 +195,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d
         *      @todo -
         */
        request->packet->code = packet->opcode;
-       request->packet->id = (data[0] << 8) | data[1];
+       request->packet->id = fr_nbo_to_uint16(data);
        request->reply->id = request->packet->id;
 
        request->packet->data = talloc_memdup(request->packet, data, data_len);

diff --git a/src/listen/radius/proto_radius_tcp.c b/src/listen/radius/proto_radius_tcp.c
index a9f6c6400a39351d6931fdadb4817c9f57083b5f..56be90e75f935b3268a5d473278b50087ad89517 100644 (file)
--- a/src/listen/radius/proto_radius_tcp.c
+++ b/src/listen/radius/proto_radius_tcp.c
@@ -151,7 +151,7 @@ static ssize_t mod_read(fr_listen_t *li, UNUSED void **packet_ctx, fr_time_t *re
        /*
         *      Figure out how large the RADIUS packet is.
         */
-       packet_len = (buffer[2] << 8) | buffer[3];
+       packet_len = fr_nbo_to_uint16(buffer + 2);
 
        /*
         *      We don't have a complete RADIUS packet.  Tell the

diff --git a/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c b/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c
index 621794653bd1aae9e0b9ac52614d6d5fd8bde125..83b61abaea037f30c1e65ffb6ca1c876b49539b1 100644 (file)
--- a/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c
+++ b/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c
@@ -622,7 +622,7 @@ failure:
         *      The MS-Length field is 5 + value_size + length
         *      of name, which is put after the response.
         */
-       length = (eap_round->response->type.data[2] << 8) | eap_round->response->type.data[3];
+       length = fr_nbo_to_uint16(eap_round->response->type.data + 2);
        if ((length < (5 + 49)) || (length > (256 + 5 + 49))) {
                REDEBUG("Response contains contradictory length %zu %d", length, 5 + 49);
                RETURN_MODULE_INVALID;

diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c
index eb664d2ac5afecc11dd2f14d5d5772ba6b9b256d..6cbb0dbbb086cecabb6cbd196fb5471e078009b7 100644 (file)
--- a/src/modules/rlm_radius/rlm_radius_udp.c
+++ b/src/modules/rlm_radius/rlm_radius_udp.c
@@ -2185,7 +2185,7 @@ static void protocol_error_reply(udp_request_t *u, udp_result_t *r, udp_handle_t
        uint32_t        response_length = 0;
        uint8_t const   *attr, *end;
 
-       end = h->buffer + ((h->buffer[2] << 8) | h->buffer[3]);
+       end = h->buffer + fr_nbo_to_uint16(h->buffer + 2);
 
        for (attr = h->buffer + RADIUS_HEADER_LENGTH;
             attr < end;

diff --git a/src/modules/rlm_wimax/rlm_wimax.c b/src/modules/rlm_wimax/rlm_wimax.c
index a729020928ceb943549dbd08101a27960a398729..77a1cbf4f779ae5ee089bcfdb1ef338618cbdeba 100644 (file)
--- a/src/modules/rlm_wimax/rlm_wimax.c
+++ b/src/modules/rlm_wimax/rlm_wimax.c
@@ -240,8 +240,7 @@ static unlang_action_t CC_HINT(nonnull) mod_post_auth(rlm_rcode_t *p_result, mod
         *      Take the 4 most significant octets.
         *      If less than 256, add 256.
         */
-       mip_spi = ((mip_rk_1[0] << 24) | (mip_rk_1[1] << 16) |
-                  (mip_rk_1[2] << 8) | mip_rk_1[3]);
+       mip_spi = fr_nbo_to_uint32(mip_rk_1);
        if (mip_spi < 256) mip_spi += 256;
 
        REDEBUG2("MIP-RK = 0x%pH", fr_box_octets(mip_rk, rk_len));

diff --git a/src/protocols/radius/base.c b/src/protocols/radius/base.c
index afcc9b92af867697f3a92c93ce579e074ce28826..8f7efdc95ada05cf2e3165c15c04657363da3638 100644 (file)
--- a/src/protocols/radius/base.c
+++ b/src/protocols/radius/base.c
@@ -278,7 +278,7 @@ int fr_radius_sign(uint8_t *packet, uint8_t const *original,
                   uint8_t const *secret, size_t secret_len)
 {
        uint8_t         *msg, *end;
-       size_t          packet_len = (packet[2] << 8) | packet[3];
+       size_t          packet_len = fr_nbo_to_uint16(packet + 2);
 
        /*
         *      No real limit on secret length, this is just
@@ -465,7 +465,7 @@ bool fr_radius_ok(uint8_t const *packet, size_t *packet_len_p,
         *      i.e. We've received 128 bytes, and the packet header
         *      says it's 256 bytes long.
         */
-       totallen = (packet[2] << 8) | packet[3];
+       totallen = fr_nbo_to_uint16(packet + 2);
 
        /*
         *      Code of 0 is not understood.
@@ -710,7 +710,7 @@ int fr_radius_verify(uint8_t *packet, uint8_t const *original,
        bool found_ma;
        int rcode;
        uint8_t *msg, *end;
-       size_t packet_len = (packet[2] << 8) | packet[3];
+       size_t packet_len = fr_nbo_to_uint16(packet + 2);
        uint8_t request_authenticator[RADIUS_AUTH_VECTOR_LENGTH];
        uint8_t message_authenticator[RADIUS_AUTH_VECTOR_LENGTH];
 

diff --git a/src/protocols/radius/packet.c b/src/protocols/radius/packet.c
index 7cbedb3644ab33e458f957bab9beab19cd2292b3..25a94199a46d5acd0c24e07125a56d6e7d56ea1b 100644 (file)
--- a/src/protocols/radius/packet.c
+++ b/src/protocols/radius/packet.c
@@ -531,7 +531,7 @@ void _fr_radius_packet_log_hex(fr_log_t const *log, fr_radius_packet_t const *pa
        }
 
        fr_log(log, L_DBG, file, line, "  Id       : %u", packet->data[1]);
-       fr_log(log, L_DBG, file, line, "  Length   : %u", ((packet->data[2] << 8) | (packet->data[3])));
+       fr_log(log, L_DBG, file, line, "  Length   : %u", fr_nbo_to_uint16(packet->data + 2));
        fr_log(log, L_DBG, file, line, "  Vector   : %pH", fr_box_octets(packet->data + 4, RADIUS_AUTH_VECTOR_LENGTH));
 
        if (packet->data_len <= 20) return;
@@ -552,7 +552,7 @@ void _fr_radius_packet_log_hex(fr_log_t const *log, fr_radius_packet_t const *pa
               p = buffer + strlen(buffer);
                if ((attr[0] == FR_VENDOR_SPECIFIC) &&
                    (attr[1] > 6)) {
-                       vendor = (attr[2] << 24) | (attr[3] << 16) | (attr[4] << 8) | attr[5];
+                       vendor = fr_nbo_to_uint32(attr + 2);
 
                       snprintf(p, buffer + sizeof(buffer) - p, "%02x%02x%02x%02x (%u)  ",
                                attr[2], attr[3], attr[4], attr[5], vendor);

diff --git a/src/protocols/radius/tcp.c b/src/protocols/radius/tcp.c
index d63306e4c285a827d50cb0c37b6523094984d5d3..74eea4a40ffd1aab2c1e0ff99aa635717b644205 100644 (file)
--- a/src/protocols/radius/tcp.c
+++ b/src/protocols/radius/tcp.c
@@ -88,7 +88,7 @@ int fr_tcp_read_packet(fr_radius_packet_t *packet, uint32_t max_attributes, bool
                        return 0;
                }
 
-               packet_len = (packet->vector[2] << 8) | packet->vector[3];
+               packet_len = fr_nbo_to_uint16(packet->vector + 2);
 
                if (packet_len < RADIUS_HEADER_LENGTH) {
                        fr_strerror_const("Discarding packet: Smaller than RFC minimum of 20 bytes");

diff --git a/src/protocols/vmps/vmps.c b/src/protocols/vmps/vmps.c
index b0246b6575498c372e423fca8a564323f0675afd..7ca5692527d40535a5a31e51801ce97a924cf998 100644 (file)
--- a/src/protocols/vmps/vmps.c
+++ b/src/protocols/vmps/vmps.c
@@ -119,13 +119,12 @@ bool fr_vmps_ok(uint8_t const *packet, size_t *packet_len)
                 *
                 *      It's OK for ethernet frames to be longer.
                 */
-               if ((ptr[3] != 5) &&
-                   ((ptr[4] != 0) || (ptr[5] > 250))) {
+               attrlen = fr_nbo_to_uint16(ptr + 4);
+               if ((ptr[3] != 5) && (attrlen > 250)) {
                        fr_strerror_printf("Packet contains attribute with invalid length %02x %02x", ptr[4], ptr[5]);
                        return false;
                }
 
-               attrlen = (ptr[4] << 8) | ptr[5];
                ptr += 6 + attrlen;
                data_len -= (6 + attrlen);
        }
@@ -189,8 +188,8 @@ int fr_vmps_decode(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t const *data, si
                        return -1;
                }
 
-               attr = (ptr[2] << 8) | ptr[3];
-               attr_len = (ptr[4] << 8) | ptr[5];
+               attr = fr_nbo_to_uint16(ptr + 2);
+               attr_len = fr_nbo_to_uint16(ptr + 4);
                ptr += 6;
 
                /*
@@ -401,7 +400,7 @@ ssize_t fr_vmps_packet_size(uint8_t const *data, size_t data_len)
                /*
                 *      Length of the data NOT including the header.
                 */
-               attr_len = (ptr[4] << 8) | ptr[5];
+               attr_len = fr_nbo_to_uint16(ptr + 4);
 
                ptr += 6 + attr_len;
 
@@ -490,7 +489,7 @@ void fr_vmps_print_hex(FILE *fp, uint8_t const *packet, size_t packet_len)
                memcpy(&id, attr, 4);
                id = ntohl(id);
 
-               length = (attr[4] << 8) | attr[5];
+               length = fr_nbo_to_uint16(attr + 4);
                if ((attr + length) > end) break;
 
                fprintf(fp, "\t\t%08x  %04x  ", id, length);

diff --git a/src/tests/util/radius1_test.c b/src/tests/util/radius1_test.c
index 147445114a5c0aa7dca8847c2abf1c5894de312d..a65a981df4eb0ed6366b488fb15d2e2fb4e80823 100644 (file)
--- a/src/tests/util/radius1_test.c
+++ b/src/tests/util/radius1_test.c
@@ -373,7 +373,7 @@ static void master_process(TALLOC_CTX *ctx)
                                goto discard;
                        }
 
-                       total_len = (packet[2] << 8) | packet[3];
+                       total_len = fr_nbo_to_uint16(packet + 2);
                        if (total_len < 20) {
                                MPRINT1("Master ignoring packet (header length %zu)\n", total_len);
                                goto discard;