Мы решили в меру своих сил регулярно искать и устранять потенциальные уязвимости и баги в различных проектах. Можно назвать это помощью open-source проектам. Можно — разновидностью рекламы или тестированием анализатора. Еще вариант — очередной способ привлечения внимания к вопросам качества и надёжности кода. На самом деле, не важно название, просто нам нравится это делать. Назовём это необычным хобби. Давайте посмотрим, что интересного было обнаружено в коде различных проектов на этой неделе. Мы нашли время сделать исправления и предлагаем вам ознакомиться с ними.

Для тех, кто ещё не знаком с инструментом PVS-Studio

PVS-Studio ^[1] — это инструмент, который выявляет в коде многие разновидности ошибок и уязвимостей. PVS-Studio выполняет статический анализ кода и рекомендует программисту обратить внимание на участки программы, в которых с большой вероятностью содержатся ошибки. Наилучший эффект достигается тогда, когда статический анализ выполняется регулярно. Идеологически предупреждения анализатора подобны предупреждениям компилятора. Но в отличии от компиляторов, PVS-Studio выполняет более глубокий и разносторонний анализ кода. Это позволяет ему находить ошибки в том числе и в компиляторах: GCC ^[2]; LLVM 1 ^[3], 2 ^[4], 3 ^[5]; Roslyn ^[6].

Поддерживается анализ кода на языках C, C++ и C#. Анализатор работает под управлением Windows и Linux. В Windows анализатор может интегрироваться как плагин в Visual Studio.

Для дальнейшего знакомства с анализатором, предлагаем изучить следующие материалы:

• Подробная презентация ^[7] на сайте SlideShare. В формате видео ^[8] она доступна на YouTube (47 минут).
• Статьи ^[9] о проверенных открытых проектах.
• PVS-Studio: поиск дефектов безопасности ^[10].

Уязвимости

В этом разделе приведены дефекты, которые попадают под классификацию CWE и, по сути, являются потенциальными уязвимостями. Конечно, не в каждом проекте дефекты безопасности создают какую-то практическую угрозу, но хочется продемонстрировать, что мы умеем находить подобные ситуации.

1. Clang. CWE-571 (Expression is Always True)

V768 ^[11] The enumeration constant 'S_MOVRELS_B64' is used as a variable of a Boolean-type. gcnhazardrecognizer.cpp 75

namespace AMDGPU {
  enum {
    ....
    S_MOVRELS_B64 = 4043,
    ....
  };
}

static bool isSMovRel(unsigned Opcode) {
  return
    Opcode == AMDGPU::S_MOVRELS_B32 || AMDGPU::S_MOVRELS_B64 ||
    Opcode == AMDGPU::S_MOVRELD_B32 || AMDGPU::S_MOVRELD_B64;
}

Pull Request: https://bugs.llvm.org/show_bug.cgi?id=32248 ^[12]

2. Clang. CWE-457 (Use of Uninitialized Variable)

V573 ^[13] Uninitialized variable 'BytesToDrop' was used. The variable was used to initialize itself. typerecordmapping.cpp 73

static Error mapNameAndUniqueName(....) {
  ....
  size_t BytesLeft = IO.maxFieldLength();
  if (HasUniqueName) {
    .....
    if (BytesNeeded > BytesLeft) {
      size_t BytesToDrop = (BytesNeeded - BytesLeft);
      size_t DropN = std::min(N.size(), BytesToDrop / 2);
      size_t DropU = std::min(U.size(), BytesToDrop - DropN);
      ....
    }
  } else {
    size_t BytesNeeded = Name.size() + 1;
    StringRef N = Name;
    if (BytesNeeded > BytesLeft) {
      size_t BytesToDrop = std::min(N.size(), BytesToDrop); // <=
      N = N.drop_back(BytesToDrop);
    }
    error(IO.mapStringZ(N));
  }
  ....
}

Pull Request: https://bugs.llvm.org/show_bug.cgi?id=32249 ^[14]

3. Clang. CWE-570 Expression is Always False

V517 ^[15] The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 416, 418. iteratorpastendchecker.cpp 416

bool IteratorPastEndChecker::evalCall(const CallExpr *CE,
                                      CheckerContext &C) const {
  ....
  if (FD->getIdentifier() == II_find) {
    return evalFind(C, CE);
  } else if (FD->getIdentifier() == II_find_end) {
    return evalFindEnd(C, CE);
  } else if (FD->getIdentifier() == II_find_first_of) {
    return evalFindFirstOf(C, CE);
  } else if (FD->getIdentifier() == II_find_if) {         // <=
    return evalFindIf(C, CE);
  } else if (FD->getIdentifier() == II_find_if) {         // <=
    return evalFindIf(C, CE);
  } else if (FD->getIdentifier() == II_find_if_not) {
    return evalFindIfNot(C, CE);
  } else if (FD->getIdentifier() == II_upper_bound) {
    return evalUpperBound(C, CE);
  } else if (FD->getIdentifier() == II_lower_bound) {
    return evalLowerBound(C, CE);
  } else if (FD->getIdentifier() == II_search) {
    return evalSearch(C, CE);
  } else if (FD->getIdentifier() == II_search_n) {
    return evalSearchN(C, CE);
  }
  ....
}

Pull Request: https://bugs.llvm.org/show_bug.cgi?id=32250 ^[16]

4. GCC. CWE-476 (NULL Pointer Dereference)

V595 ^[17] The 'm->component' pointer was utilized before it was verified against nullptr. Check lines: 399, 407. genmodes.c 399

static void complete_mode (struct mode_data *m)
{
  ....
  if (   m->cl == MODE_COMPLEX_INT 
      || m->cl == MODE_COMPLEX_FLOAT)
    alignment = m->component->bytesize;        // <=
  else
    alignment = m->bytesize;

  m->alignment = alignment & (~alignment + 1);

  if (m->component)                            // <=
  {
    m->next_cont = m->component->contained;
    m->component->contained = m;
  }
}

Pull Request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80049 ^[18]

5. GCC. CWE-570 (Expression is Always False)

V625 ^[19] Consider inspecting the 'for' operator. Initial and final values of the iterator are the same. sese.c 201

void free_sese_info (sese_info_p region)
{
  region->params.release ();
  region->loop_nest.release ();

  for (rename_map_t::iterator it = region->rename_map->begin();
       it != region->rename_map->begin (); ++it) // <=
    (*it).second.release();
  ....
}

Pull Request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80048 ^[20]

6. GCC. CWE-571 (Expression is Always True)

V501 ^[21] There are identical sub-expressions '!strcmp(a->v.val_vms_delta.lbl1, b->v.val_vms_delta.lbl1)' to the left and to the right of the '&&' operator. dwarf2out.c 1434

static bool dw_val_equal_p (dw_val_node *a, dw_val_node *b)
{
  ....
  switch (a->val_class)
  {
    ....
  case dw_val_class_vms_delta:
    return (   !strcmp (a->v.val_vms_delta.lbl1,
                        b->v.val_vms_delta.lbl1)
            && !strcmp (a->v.val_vms_delta.lbl1, 
                        b->v.val_vms_delta.lbl1));
    ....
  }
  ....
}

Pull Request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80051 ^[22]

7. GCC. CWE-483 (Incorrect Block Delimitation)

V640 ^[23] The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. asan.c 2582

void initialize_sanitizer_builtins (void)
{
  ....
  #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) 
  decl = add_builtin_function ("__builtin_" NAME, TYPE, ENUM, 
             BUILT_IN_NORMAL, NAME, NULL_TREE);  
  set_call_expr_flags (decl, ATTRS);          
  set_builtin_decl (ENUM, decl, true);

  #include "sanitizer.def"

  if ((flag_sanitize & SANITIZE_OBJECT_SIZE)
      && !builtin_decl_implicit_p (BUILT_IN_OBJECT_SIZE))
    DEF_SANITIZER_BUILTIN (BUILT_IN_OBJECT_SIZE, "object_size",
         BT_FN_SIZE_CONST_PTR_INT,
         ATTR_PURE_NOTHROW_LEAF_LIST)
  ....
}

Pull Request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80063 ^[24]

8. FreeBSD. CWE-467: (Use of sizeof() on a Pointer Type)

V512 ^[25] A call of the 'memset' function will lead to underflow of the buffer 'plog'. nat64lsn.c 218

struct pfloghdr {
  u_int8_t  length;
  sa_family_t  af;
  u_int8_t  action;
  u_int8_t  reason;
  char    ifname[IFNAMSIZ];
  char    ruleset[PFLOG_RULESET_NAME_SIZE];
  u_int32_t  rulenr;
  u_int32_t  subrulenr;
  uid_t    uid;
  pid_t    pid;
  uid_t    rule_uid;
  pid_t    rule_pid;
  u_int8_t  dir;
  u_int8_t  pad[3];
};

static void
nat64lsn_log(struct pfloghdr *plog, ....)
{
  memset(plog, 0, sizeof(plog));        // <=
  plog->length = PFLOG_REAL_HDRLEN;
  plog->af = family;
  plog->action = PF_NAT;
  plog->dir = PF_IN;
  plog->rulenr = htonl(n);
  plog->subrulenr = htonl(sn);
  plog->ruleset[0] = '';
  strlcpy(plog->ifname, "NAT64LSN", sizeof(plog->ifname));
  ipfw_bpf_mtap2(plog, PFLOG_HDRLEN, m);
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217738 ^[26]

9. FreeBSD. CWE-570 (Expression is Always False)

V517 ^[15] The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 102, 109. dtrace_debug.c 102

static void
dtrace_debug_output(void)
{
  ....
  if (d->first < d->next) {
    char *p1 = dtrace_debug_bufr;
    count = (uintptr_t) d->next - (uintptr_t) d->first;
    for (p = d->first; p < d->next; p++)
      *p1++ = *p;
  } else if (d->next > d->first) {
    char *p1 = dtrace_debug_bufr;
    count = (uintptr_t) d->last - (uintptr_t) d->first;
    for (p = d->first; p < d->last; p++)
      *p1++ = *p;
    count += (uintptr_t) d->next - (uintptr_t) d->bufr;
    for (p = d->bufr; p < d->next; p++)
      *p1++ = *p;
  }
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217739 ^[27]

10. FreeBSD. CWE-571 (Expression is Always True)

V547 ^[28] Expression 'cfgflags >= 0 || cfgflags <= 3' is always true. hwpmc_piv.c 812

V547 ^[28] Expression 'cfgflags >= 0 || cfgflags <= 3' is always true. hwpmc_piv.c 838

static int
p4_config_pmc(int cpu, int ri, struct pmc *pm)
{
  ....
  int cfgflags, cpuflag;
  ....
  KASSERT(cfgflags >= 0 || cfgflags <= 3,
      ("[p4,%d] illegal cfgflags cfg=%d on cpu=%d ri=%d",
    __LINE__, cfgflags, cpu, ri));
  ....
  KASSERT(cfgflags >= 0 || cfgflags <= 3,
      ("[p4,%d] illegal runcount cfg=%d on cpu=%d ri=%d",
    __LINE__, cfgflags, cpu, ri));
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217741 ^[29]

11. FreeBSD. CWE-570 (Expression is Always False)

V547 ^[28] Expression is always false. scif_sas_controller.c 531

....
U16  max_ncq_depth;
....
SCI_STATUS scif_user_parameters_set(
   SCI_CONTROLLER_HANDLE_T   controller,
   SCIF_USER_PARAMETERS_T  * scif_parms
)
{
  ....
   if (scif_parms->sas.max_ncq_depth < 1 &&
       scif_parms->sas.max_ncq_depth > 32)
     return SCI_FAILURE_INVALID_PARAMETER_VALUE;
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217742 ^[30]

12. FreeBSD. CWE-571: (Expression is Always True)

V547 ^[28] Expression 'cdb[0] != 0x28 || cdb[0] != 0x2A' is always true. Probably the '&&' operator should be used here. mfi_tbolt.c 1110

int
mfi_tbolt_send_frame(struct mfi_softc *sc, struct mfi_command *cm)
{
  ....
  uint8_t *cdb;
  ....
  /* check for inquiry commands coming from CLI */
  if (cdb[0] != 0x28 || cdb[0] != 0x2A) {
    if ((req_desc = mfi_tbolt_build_mpt_cmd(sc, cm)) == NULL) {
      device_printf(sc->mfi_dev, "Mapping from MFI "
                                 "to MPT Failed n");
      return 1;
    }
  }
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217743 ^[31]

13. FreeBSD. CWE-571 (Expression is Always True)

V560 ^[32] A part of conditional expression is always true: 0x2002. sampirsp.c 7224

#define OSSA_MPI_ENC_ERR_ILLEGAL_DEK_PARAM            0x2001
#define OSSA_MPI_ERR_DEK_MANAGEMENT_DEK_UNWRAP_FAIL   0x2002

GLOBAL bit32 mpiDekManagementRsp(
  agsaRoot_t               *agRoot,
  agsaDekManagementRsp_t   *pIomb
  )
{
  ....
  if (status == OSSA_MPI_ENC_ERR_ILLEGAL_DEK_PARAM ||
      OSSA_MPI_ERR_DEK_MANAGEMENT_DEK_UNWRAP_FAIL)
  {
    agEvent.eq = errorQualifier;
  }
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217745 ^[33]

14. FreeBSD. CWE-571 (Expression is Always True)

V560 ^[32] A part of conditional expression is always true: 0x7dac. t4_main.c 8001

#define A_TP_KEEP_INTVL 0x7dac

static int
sysctl_tp_timer(SYSCTL_HANDLER_ARGS)
{
  struct adapter *sc = arg1;
  int reg = arg2;
  u_int tre;
  u_long tp_tick_us, v;
  u_int cclk_ps = 1000000000 / sc->params.vpd.cclk;

  MPASS(reg == A_TP_RXT_MIN || reg == A_TP_RXT_MAX ||
      reg == A_TP_PERS_MIN || reg == A_TP_PERS_MAX ||
      reg == A_TP_KEEP_IDLE || A_TP_KEEP_INTVL ||
      reg == A_TP_INIT_SRTT || reg == A_TP_FINWAIT2_TIMER);
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217746 ^[34]

15. FreeBSD. CWE-476 (NULL Pointer Dereference)

V595 ^[17] The 'mc' pointer was utilized before it was verified against nullptr. Check lines: 2954, 2955. mly.c 2954

static int
mly_user_command(struct mly_softc *sc, struct mly_user_command *uc)
{
  struct mly_command  *mc;
  ....
  if (mc->mc_data != NULL)           // <=
    free(mc->mc_data, M_DEVBUF);     // <=
  if (mc != NULL) {                  // <=
    MLY_LOCK(sc);
    mly_release_command(mc);
    MLY_UNLOCK(sc);
  }
  return(error);
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217747 ^[35]

16. FreeBSD. CWE-563 (Assignment to Variable without Use ('Unused Variable'))

V519 ^[36] The 'vf->flags' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 5992, 5994. if_ix.c 5994

static int
ixgbe_add_vf(device_t dev, u16 vfnum, const nvlist_t *config)
{
  ....
  if (nvlist_exists_binary(config, "mac-addr")) {
    mac = nvlist_get_binary(config, "mac-addr", NULL);
    bcopy(mac, vf->ether_addr, ETHER_ADDR_LEN);
    if (nvlist_get_bool(config, "allow-set-mac"))
      vf->flags |= IXGBE_VF_CAP_MAC;
  } else
    /*
     * If the administrator has not specified a MAC address then
     * we must allow the VF to choose one.
     */
    vf->flags |= IXGBE_VF_CAP_MAC;

  vf->flags = IXGBE_VF_ACTIVE;
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217748 ^[37]

17. FreeBSD. CWE-563 (Assignment to Variable without Use ('Unused Variable'))

V519 ^[36] The 'pmuctrl' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 2025, 2026. bhnd_pmu_subr.c 2026

static void
bhnd_pmu1_pllinit0(struct bhnd_pmu_softc *sc, uint32_t xtal)
{
  uint32_t pmuctrl;
  ....
  /* Write XtalFreq. Set the divisor also. */
  pmuctrl = BHND_PMU_READ_4(sc, BHND_PMU_CTRL);
  pmuctrl = ~(BHND_PMU_CTRL_ILP_DIV_MASK |
              BHND_PMU_CTRL_XTALFREQ_MASK);
  pmuctrl |= BHND_PMU_SET_BITS(((xt->fref + 127) / 128) - 1,
             BHND_PMU_CTRL_ILP_DIV);
  pmuctrl |= BHND_PMU_SET_BITS(xt->xf, BHND_PMU_CTRL_XTALFREQ);
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217782 ^[38]

18. FreeBSD. CWE-561 (Dead Code)

V779 ^[39] Unreachable code detected. It is possible that an error is present. if_wi_pci.c 258

static int
wi_pci_resume(device_t dev)
{
  struct wi_softc  *sc = device_get_softc(dev);
  struct ieee80211com *ic = &sc->sc_ic;

  WI_LOCK(sc);
  if (sc->wi_bus_type != WI_BUS_PCI_NATIVE) {
    return (0);                                 // <=
    WI_UNLOCK(sc);                              // <=
  }
  if (ic->ic_nrunning > 0)
    wi_init(sc);
  WI_UNLOCK(sc);
  return (0);
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217784 ^[40]

19. FreeBSD. CWE-561 (Dead Code)

V779 ^[39] Unreachable code detected. It is possible that an error is present. mpr.c 1329

void panic(const char *a) __dead2;

static int
mpr_alloc_requests(struct mpr_softc *sc)
{
  ....
  else {
    panic("failed to allocate command %dn", i);
    sc->num_reqs = i;
    break;
  }
  ....
}

Pull Request: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=217785 ^[41]

Прочие ошибки

1. GCC

V590 ^[42] Consider inspecting this expression. The expression is excessive or contains a misprint. genmatch.c 3829

const cpp_token * parser::next ()
{
  const cpp_token *token;
  do
  {
    token = cpp_get_token (r);
  }
  while (   token->type == CPP_PADDING
         && token->type != CPP_EOF);    // <=
  return token;
}

Pull Request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80050 ^[43]

2. Clang

V501 ^[21] There are identical sub-expressions 'RA.getSubReg() != 0' to the left and to the right of the '||' operator. hexagonearlyifconv.cpp 485

unsigned HexagonEarlyIfConversion::computePhiCost(....) const {
  ....
  const MachineOperand &RA = MI.getOperand(1);
  const MachineOperand &RB = MI.getOperand(3);
  assert(RA.isReg() && RB.isReg());
  // Must have a MUX if the phi uses a subregister.
  if (RA.getSubReg() != 0 || RA.getSubReg() != 0) {
    Cost++;
    continue;
  }
  ....
}

Pull Request: https://bugs.llvm.org/show_bug.cgi?id=32265 ^[44]

Заключение

Предлагаем скачать анализатор PVS-Studio и попробовать проверить ваш проект:

• Скачать PVS-Studio для Windows ^[45]
• Скачать PVS-Studio для Linux ^[46]

Для снятия ограничения ^[47] демонстрационной версии, вы можете написать ^[48] нам, и мы отправим вам временный ключ.

Для быстрого знакомства с анализатором, вы можете воспользоваться утилитами, отслеживающими запуски компилятора и собирающие для проверки всю необходимую информацию. См. описание утилиты CLMonitoring ^[49] и pvs-studio-analyzer ^[50]. Если вы работаете с классическим типом проекта в Visual Studio, то всё ещё проще: достаточно выбрать в меню PVS-Studio команду «Check Solution».

Автор: PVS-Studio

Источник ^[51]