From: Juergen Perlinger Date: Sun, 26 Feb 2017 08:20:56 +0000 (+0000) Subject: [Sec 3393] clang scan-build findings X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=609fa8d28de76327abae9e5d5adfe5553e48bf84;p=thirdparty%2Fntp.git [Sec 3393] clang scan-build findings bk: 58b28fe8eOyaVjdyTxHMDIjrhGTt5g --- diff --git a/ChangeLog b/ChangeLog index fea59795f..9369a98b9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +--- +* [Sec 3393] clang scan-build findings + --- (4.2.8p10) diff --git a/libntp/ntp_intres.c b/libntp/ntp_intres.c index 7aa288af5..9fc3815ef 100644 --- a/libntp/ntp_intres.c +++ b/libntp/ntp_intres.c @@ -410,7 +410,7 @@ blocking_getaddrinfo( cp += sizeof(*ai); /* transform ai_canonname into offset */ - if (NULL != serialized_ai->ai_canonname) { + if (NULL != ai->ai_canonname) { serialized_ai->ai_canonname = (char *)canons_octets; canons_octets += strlen(ai->ai_canonname) + 1; } diff --git a/ntpd/ntp_config.c b/ntpd/ntp_config.c index 9748afab9..ec7613abb 100644 --- a/ntpd/ntp_config.c +++ b/ntpd/ntp_config.c @@ -367,7 +367,23 @@ static int getnetnum(const char *num, sockaddr_u *addr, int complain, #endif +#if defined(__GNUC__) /* this covers CLANG, too */ +static void __attribute__((noreturn,format(printf,1,2))) fatal_error(const char *fmt, ...) +#elif defined(_MSC_VER) +static void __declspec(noreturn) fatal_error(const char *fmt, ...) +#else +static void fatal_error(const char *fmt, ...) +#endif +{ + va_list va; + + va_start(va, fmt); + mvsyslog(LOG_EMERG, fmt, va); + va_end(va); + _exit(1); +} + /* FUNCTIONS FOR INITIALIZATION * ---------------------------- */ @@ -1841,16 +1857,12 @@ config_auth( /* Crypto Command */ #ifdef AUTOKEY -# ifdef __GNUC__ - item = -1; /* quiet warning */ -# endif my_val = HEAD_PFIFO(ptree->auth.crypto_cmd_list); for (; my_val != NULL; my_val = my_val->link) { switch (my_val->attr) { default: - INSIST(0); - break; + fatal_error("config_auth: attr-token=%d", my_val->attr); case T_Host: item = CRYPTO_CONF_PRIV; @@ -2036,7 +2048,7 @@ config_tos( msyslog(LOG_WARNING, "Using maximum tos ceiling %d, %d requested", STRATUM_UNSPEC - 1, (int)val); - val = STRATUM_UNSPEC - 1; + tos->value.d = STRATUM_UNSPEC - 1; } else if (val < 1) { msyslog(LOG_WARNING, "Using minimum tos floor %d, %d requested", @@ -2079,9 +2091,7 @@ config_tos( switch(tos->attr) { default: - item = -1; /* quiet warning */ - INSIST(0); - break; + fatal_error("config-tos: attr-token=%d", tos->attr); case T_Bcpollbstep: item = PROTO_BCPOLLBSTEP; @@ -2230,8 +2240,7 @@ config_monitor( switch (my_opts->value.i) { default: - INSIST(0); - break; + fatal_error("config-monitor: type-token=%d", my_opts->value.i); case T_None: filegen_type = FILEGEN_NONE; @@ -2467,8 +2476,7 @@ config_access( switch (curr_flag->i) { default: - INSIST(0); - break; + fatal_error("config-access: flag-type-token=%d", curr_flag->i); case T_Ntpport: mflags |= RESM_NTPONLY; @@ -2698,8 +2706,7 @@ config_rlimit( switch (rlimit_av->attr) { default: - INSIST(0); - break; + fatal_error("config-rlimit: value-token=%d", rlimit_av->attr); case T_Memlock: /* What if we HAVE_OPT(SAVECONFIGQUIT) ? */ @@ -2772,16 +2779,12 @@ config_tinker( attr_val * tinker; int item; -#ifdef __GNUC__ - item = -1; /* quiet warning */ -#endif tinker = HEAD_PFIFO(ptree->tinker); for (; tinker != NULL; tinker = tinker->link) { switch (tinker->attr) { default: - INSIST(0); - break; + fatal_error("config_tinker: attr-token=%d", tinker->attr); case T_Allan: item = LOOP_ALLAN; @@ -2888,16 +2891,7 @@ config_nic_rules( switch (curr_node->match_class) { default: -#ifdef __GNUC__ - /* - * this assignment quiets a gcc "may be used - * uninitialized" warning and is here for no - * other reason. - */ - match_type = MATCH_ALL; -#endif - INSIST(FALSE); - break; + fatal_error("config_nic_rules: match-class-token=%d", curr_node->match_class); case 0: /* @@ -2948,16 +2942,7 @@ config_nic_rules( switch (curr_node->action) { default: -#ifdef __GNUC__ - /* - * this assignment quiets a gcc "may be used - * uninitialized" warning and is here for no - * other reason. - */ - action = ACTION_LISTEN; -#endif - INSIST(FALSE); - break; + fatal_error("config_nic_rules: action-token=%d", curr_node->action); case T_Listen: action = ACTION_LISTEN; @@ -3145,8 +3130,7 @@ config_logconfig( ntp_syslogmask = get_logmask(my_lc->value.s); break; default: - INSIST(0); - break; + fatal_error("config-logconfig: modifier='%c'", my_lc->attr); } } } @@ -3796,8 +3780,7 @@ peerflag_bits( switch (option->value.i) { default: - INSIST(0); - break; + fatal_error("peerflag_bits: option-token=%d", option->value.i); case T_Autokey: peerflags |= FLAG_SKEY; @@ -5099,8 +5082,7 @@ ntp_rlimit( # endif /* RLIMIT_STACK */ default: - INSIST(!"Unexpected setrlimit() case!"); - break; + fatal_error("ntp_rlimit: unexpected RLIMIT case: %d", rl_what); } } #endif /* HAVE_SETRLIMIT */ diff --git a/ntpd/ntp_control.c b/ntpd/ntp_control.c index 6324daafe..558627010 100644 --- a/ntpd/ntp_control.c +++ b/ntpd/ntp_control.c @@ -2088,7 +2088,7 @@ ctl_putsys( buffp = buf; buffend = buf + sizeof(buf); - if (strlen(sys_var[CS_VARLIST].text) + 4 > buffend - buffp) + if (strlen(sys_var[CS_VARLIST].text) > (sizeof(buf) - 4)) break; /* really long var name */ snprintf(buffp, sizeof(buf), "%s=\"",sys_var[CS_VARLIST].text); diff --git a/ntpd/ntp_scanner.c b/ntpd/ntp_scanner.c index 631c218b1..6cfbeef40 100644 --- a/ntpd/ntp_scanner.c +++ b/ntpd/ntp_scanner.c @@ -892,7 +892,6 @@ yylex(void) } } - instring = FALSE; if (FOLLBY_STRING == followedby) followedby = FOLLBY_TOKEN; diff --git a/ntpd/refclock_gpsdjson.c b/ntpd/refclock_gpsdjson.c index 7c4931149..00cd3fc53 100644 --- a/ntpd/refclock_gpsdjson.c +++ b/ntpd/refclock_gpsdjson.c @@ -2112,13 +2112,15 @@ save_ltc( clockprocT * const pp, const char * const tc) { - size_t len; - - len = (tc) ? strlen(tc) : 0; - if (len >= sizeof(pp->a_lastcode)) - len = sizeof(pp->a_lastcode) - 1; + size_t len = 0; + + if (tc) { + len = strlen(tc); + if (len >= sizeof(pp->a_lastcode)) + len = sizeof(pp->a_lastcode) - 1; + memcpy(pp->a_lastcode, tc, len); + } pp->lencode = (u_short)len; - memcpy(pp->a_lastcode, tc, len); pp->a_lastcode[len] = '\0'; } diff --git a/ntpd/refclock_jjy.c b/ntpd/refclock_jjy.c index 73ff7bc02..22636a0af 100644 --- a/ntpd/refclock_jjy.c +++ b/ntpd/refclock_jjy.c @@ -744,8 +744,8 @@ jjy_receive ( struct recvbuf *rbufp ) } iCopyLen = ( iLen <= sizeof(sLogText)-1 ? iLen : sizeof(sLogText)-1 ) ; - strncpy( sLogText, pBuf, iCopyLen ) ; - sLogText[iCopyLen] = 0 ; + memcpy( sLogText, pBuf, iCopyLen ) ; + sLogText[iCopyLen] = '\0' ; jjy_write_clockstats( peer, JJY_CLOCKSTATS_MARK_RECEIVE, sLogText ) ; switch ( up->unittype ) { diff --git a/ntpd/refclock_nmea.c b/ntpd/refclock_nmea.c index b1ea2946b..19bdeeb3b 100644 --- a/ntpd/refclock_nmea.c +++ b/ntpd/refclock_nmea.c @@ -810,9 +810,6 @@ nmea_receive( ZERO(tofs); ZERO(date); ZERO(gpsw); - sentence = 0; // Should never be needed. - rc_date = 0; // Should never be needed. - rc_time = 0; // Should never be needed. /* * Read the timecode and timestamp, then initialise field diff --git a/ntpd/refclock_oncore.c b/ntpd/refclock_oncore.c index 30924b8bb..88d310543 100644 --- a/ntpd/refclock_oncore.c +++ b/ntpd/refclock_oncore.c @@ -2514,8 +2514,6 @@ oncore_msg_Bl( WARN_MINUS } warn; - day_now = day_lsf = 0; - cp = NULL; /* keep gcc happy */ subframe = buf[6] & 017; valid = (buf[6] >> 4) & 017; @@ -2590,6 +2588,9 @@ oncore_msg_Bl( instance->peer->leap = LEAP_ADDSECOND; cp = "Set peer.leap to LEAP_ADDSECOND"; break; + default: + cp = NULL; + break; } oncore_log(instance, LOG_NOTICE, cp); @@ -3379,7 +3380,6 @@ oncore_check_antenna( { enum antenna_state antenna; /* antenna state */ - antenna = instance->ant_state; if (instance->chan == 12) antenna = (instance->BEHa[130] & 0x6 ) >> 1; else diff --git a/ntpd/refclock_parse.c b/ntpd/refclock_parse.c index 0697f3980..cf81e40e6 100644 --- a/ntpd/refclock_parse.c +++ b/ntpd/refclock_parse.c @@ -3675,8 +3675,7 @@ parse_control( } } - tt = ap(start, LEN_STATES, tt, - "; running time: %s\"", l_mktime(sum)); + ap(start, LEN_STATES, tt, "; running time: %s\"", l_mktime(sum)); tt = add_var(&out->kv_list, 32, RO); snprintf(tt, 32, "refclock_id=\"%s\"", parse->parse_type->cl_id); @@ -4248,13 +4247,11 @@ mk_utcinfo( { time_t t_ls; struct tm *tm; - int n = 0; + int nc; if (wnlsf < GPSWRAP) wnlsf += GPSWEEKS; - - if (wnt < GPSWRAP) - wnt += GPSWEEKS; + /* 'wnt' not used here: would need the same treatment as 'wnlsf */ t_ls = (time_t) wnlsf * SECSPERWEEK + (time_t) dn * SECSPERDAY @@ -4267,13 +4264,20 @@ mk_utcinfo( return; } - n += snprintf( t, size, "UTC offset transition from %is to %is due to leap second %s", + nc = snprintf( t, size, "UTC offset transition from %is to %is due to leap second %s", dtls, dtlsf, ( dtls < dtlsf ) ? "insertion" : "deletion" ); - n += snprintf( t + n, size - n, " at UTC midnight at the end of %s, %04i-%02i-%02i", + if (nc < 0) + nc = strlen(t); + else if (nc > size) + nc = size; + + snprintf( t + nc, size - nc, " at UTC midnight at the end of %s, %04i-%02i-%02i", daynames[tm->tm_wday], tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday ); } else + { snprintf( t, size, "UTC offset parameter: %is, no leap second announced.\n", dtls ); + } } diff --git a/ntpdate/ntpdate.c b/ntpdate/ntpdate.c index be39cb030..14cf22b5d 100644 --- a/ntpdate/ntpdate.c +++ b/ntpdate/ntpdate.c @@ -955,6 +955,8 @@ clock_filter( register int i, j; int ord[NTP_SHIFT]; + INSIST((0 < sys_samples) && (sys_samples <= NTP_SHIFT)); + /* * Sort indices into increasing delay order */ diff --git a/ntpdc/ntpdc.c b/ntpdc/ntpdc.c index 69fe6a5e8..0375d3681 100644 --- a/ntpdc/ntpdc.c +++ b/ntpdc/ntpdc.c @@ -649,7 +649,7 @@ getresponse( todiff = (((uint32_t)time(NULL)) - tobase) & 0x7FFFFFFFu; if ((n > 0) && (todiff > tospan)) { n = recv(sockfd, (char *)&rpkt, sizeof(rpkt), 0); - n = 0; /* faked timeout return from 'select()'*/ + n -= n; /* faked timeout return from 'select()'*/ } if (n == 0) { diff --git a/ntpq/ntpq.c b/ntpq/ntpq.c index bfc5fa9cf..547f3cc5d 100644 --- a/ntpq/ntpq.c +++ b/ntpq/ntpq.c @@ -776,31 +776,42 @@ dump_hex_printable( size_t len ) { - const char * cdata; - const char * rowstart; - size_t idx; - size_t rowlen; - u_char uch; - - cdata = data; - while (len > 0) { - rowstart = cdata; - rowlen = min(16, len); - for (idx = 0; idx < rowlen; idx++) { - uch = *(cdata++); - printf("%02x ", uch); - } - for ( ; idx < 16 ; idx++) - printf(" "); - cdata = rowstart; - for (idx = 0; idx < rowlen; idx++) { - uch = *(cdata++); - printf("%c", (isprint(uch)) - ? uch - : '.'); - } - printf("\n"); + /* every line shows at most 16 bytes, so we need a buffer of + * 4 * 16 (2 xdigits, 1 char, one sep for xdigits) + * + 2 * 1 (block separators) + * + + + *--------------- + * 68 bytes + */ + static const char s_xdig[16] = "0123456789ABCDEF"; + + char lbuf[68]; + int ch, rowlen; + const u_char * cdata = data; + char *xptr, *pptr; + + while (len) { + memset(lbuf, ' ', sizeof(lbuf)); + xptr = lbuf; + pptr = lbuf + 3*16 + 2; + + rowlen = (len > 16) ? 16 : (int)len; len -= rowlen; + + do { + ch = *cdata++; + + *xptr++ = s_xdig[ch >> 4 ]; + *xptr++ = s_xdig[ch & 0x0F]; + if (++xptr == lbuf + 3*8) + ++xptr; + + *pptr++ = isprint(ch) ? (char)ch : '.'; + } while (--rowlen); + + *pptr++ = '\n'; + *pptr = '\0'; + fputs(lbuf, stdout); } } @@ -862,6 +873,9 @@ getresponse( uint32_t tospan; /* timeout span (max delay) */ uint32_t todiff; /* current delay */ + memset(offsets, 0, sizeof(offsets)); + memset(counts , 0, sizeof(counts )); + /* * This is pretty tricky. We may get between 1 and MAXFRAG packets * back in response to the request. We peel the data out of @@ -922,10 +936,12 @@ getresponse( todiff = (((uint32_t)time(NULL)) - tobase) & 0x7FFFFFFFu; if ((n > 0) && (todiff > tospan)) { n = recv(sockfd, (char *)&rpkt, sizeof(rpkt), 0); - n = 0; /* faked timeout return from 'select()'*/ + n -= n; /* faked timeout return from 'select()', + * execute RMW cycle on 'n' + */ } - if (n == 0) { + if (n <= 0) { /* * Timed out. Return what we have */ @@ -960,7 +976,7 @@ getresponse( } n = recv(sockfd, (char *)&rpkt, sizeof(rpkt), 0); - if (n == -1) { + if (n < 0) { warning("read"); return -1; }