#1970 Dereference after a NULL check in sshsrv_cmd.c
Closed: Fixed None Opened 6 years ago by jhrozek.

668static errno_t
669ssh_cmd_parse_request(struct ssh_cmd_ctx *cmd_ctx)
670{
671    struct cli_ctx *cctx = cmd_ctx->cctx;
672    struct ssh_ctx *ssh_ctx = talloc_get_type(cctx->rctx->pvt_ctx,
673                                              struct ssh_ctx);
674    errno_t ret;
675    uint8_t *body;
676    size_t body_len;
677    size_t c = 0;
678    uint32_t flags;
679    uint32_t name_len;
680    char *name;
681    uint32_t alias_len;
682    char *alias = NULL;
683    uint32_t domain_len;
684    char *domain = cctx->rctx->default_domain;
685
686    sss_packet_get_body(cctx->creq->in, &body, &body_len);
687

1. Condition "c + 4UL /* sizeof (uint32_t) */ > body_len", taking false branch

2. Condition "4UL /* (size_t)sizeof (uint32_t) */ > 18446744073709551615UL /* (size_t)-1 */ - (size_t)c", taking false branch
688    SAFEALIGN_COPY_UINT32_CHECK(&flags, body+c, body_len, &c);

3. Condition "flags & 4294967292U /* ~((uint32_t)3) */", taking false branch
689    if (flags & ~(uint32_t)SSS_SSH_REQ_MASK) {
690        DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid flags received [0x%x]\n", flags));
691        return EINVAL;
692    }
693

4. Condition "c + 4UL /* sizeof (uint32_t) */ > body_len", taking false branch

5. Condition "4UL /* (size_t)sizeof (uint32_t) */ > 18446744073709551615UL /* (size_t)-1 */ - (size_t)c", taking false branch
694    SAFEALIGN_COPY_UINT32_CHECK(&name_len, body+c, body_len, &c);

6. Condition "name_len == 0", taking false branch

7. Condition "name_len > body_len - c", taking false branch
695    if (name_len == 0 || name_len > body_len - c) {
696        DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid name length\n"));
697        return EINVAL;
698    }
699
700    name = (char *)(body+c);

8. Condition "!sss_utf8_check((uint8_t const *)name, name_len - 1)", taking false branch

9. Condition "name[name_len - 1] != 0", taking false branch
701    if (!sss_utf8_check((const uint8_t *)name, name_len-1) ||
702            name[name_len-1] != 0) {
703        DEBUG(SSSDBG_CRIT_FAILURE, ("Name is not valid UTF-8 string\n"));
704        return EINVAL;
705    }
706    c += name_len;
707

10. Condition "flags & 1", taking true branch
708    if (flags & SSS_SSH_REQ_ALIAS) {

11. Condition "c + 4UL /* sizeof (uint32_t) */ > body_len", taking false branch

12. Condition "4UL /* (size_t)sizeof (uint32_t) */ > 18446744073709551615UL /* (size_t)-1 */ - (size_t)c", taking false branch
709        SAFEALIGN_COPY_UINT32_CHECK(&alias_len, body+c, body_len, &c);

13. Condition "alias_len == 0", taking false branch

14. Condition "alias_len > body_len - c", taking false branch
710        if (alias_len == 0 || alias_len > body_len - c) {
711            DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid alias length\n"));
712            return EINVAL;
713        }
714
715        alias = (char *)(body+c);

15. Condition "!sss_utf8_check((uint8_t const *)alias, alias_len - 1)", taking false branch

16. Condition "alias[alias_len - 1] != 0", taking false branch
716        if (!sss_utf8_check((const uint8_t *)alias, alias_len-1) ||
717                alias[alias_len-1] != 0) {
718            DEBUG(SSSDBG_CRIT_FAILURE, ("Alias is not valid UTF-8 string\n"));
719            return EINVAL;
720        }
721        c += alias_len;
722    }
723

17. Condition "flags & 2", taking true branch
724    if (flags & SSS_SSH_REQ_DOMAIN) {

18. Condition "c + 4UL /* sizeof (uint32_t) */ > body_len", taking false branch

19. Condition "4UL /* (size_t)sizeof (uint32_t) */ > 18446744073709551615UL /* (size_t)-1 */ - (size_t)c", taking false branch
725        SAFEALIGN_COPY_UINT32_CHECK(&domain_len, body+c, body_len, &c);

20. Condition "domain_len > 0", taking true branch
726        if (domain_len > 0) {

21. Condition "domain_len > body_len - c", taking false branch
727            if (domain_len > body_len - c) {
728                DEBUG(SSSDBG_CRIT_FAILURE, ("Invalid domain length\n"));
729                return EINVAL;
730            }
731
732            domain = (char *)(body+c);

22. Condition "!sss_utf8_check((uint8_t const *)domain, domain_len - 1)", taking false branch

23. Condition "domain[domain_len - 1] != 0", taking false branch
733            if (!sss_utf8_check((const uint8_t *)domain, domain_len-1) ||
734                    domain[domain_len-1] != 0) {
735                DEBUG(SSSDBG_CRIT_FAILURE,
736                      ("Domain is not valid UTF-8 string\n"));
737                return EINVAL;
738            }
739            c += domain_len;
740        }
741

24. Condition "debug_level & __debug_macro_newlevel", taking true branch

25. Condition "debug_timestamps", taking true branch

26. Condition "debug_microseconds", taking true branch

27. Falling through to end of if statement

28. Falling through to end of if statement

29. Condition "domain", taking true branch
742        DEBUG(SSSDBG_TRACE_FUNC,
743              ("Requested domain [%s]\n", domain ? domain : "<ALL>"));

30. Falling through to end of if statement
744    } else {
745        DEBUG(SSSDBG_TRACE_FUNC, ("Splitting domain from name [%s]\n", name));
746
747        ret = sss_parse_name(cmd_ctx, ssh_ctx->snctx, name,
748                             &cmd_ctx->domname, &cmd_ctx->name);
749        if (ret != EOK) {
750            DEBUG(SSSDBG_OP_FAILURE, ("Invalid name received [%s]\n", name));
751            return ENOENT;
752        }
753
754        name = cmd_ctx->name;
755    }
756

31. Condition "cmd_ctx->is_user", taking true branch

32. Condition "cmd_ctx->domname == NULL", taking false branch
757    if (cmd_ctx->is_user && cmd_ctx->domname == NULL) {
758        DEBUG(SSSDBG_TRACE_FUNC,
759              ("Parsing name [%s][%s]\n", name, domain ? domain : "<ALL>"));
760
761        ret = sss_parse_name_for_domains(cmd_ctx, cctx->rctx->domains,
762                                         domain, name,
763                                         &cmd_ctx->domname,
764                                         &cmd_ctx->name);
765        if (ret != EOK) {
766            DEBUG(SSSDBG_OP_FAILURE,
767                  ("Invalid name received [%s]\n", name));
768            return ENOENT;
769        }

33. Condition "cmd_ctx->name == NULL", taking true branch

34. var_compare_op: Comparing "cmd_ctx->name" to null implies that "cmd_ctx->name" might be null.

35. Condition "cmd_ctx->domname == NULL", taking false branch
770    } else if (cmd_ctx->name == NULL && cmd_ctx->domname == NULL) {
771        cmd_ctx->name = talloc_strdup(cmd_ctx, name);
772        if (!cmd_ctx->name) return ENOMEM;
773
774        if (domain != NULL) {
775            cmd_ctx->domname = talloc_strdup(cmd_ctx, domain);
776            if (!cmd_ctx->domname) return ENOMEM;
777        }
778    }
779

36. Condition "alias != NULL", taking true branch

CID 11647: Dereference after null check (FORWARD_NULL)37. var_deref_model: Passing null pointer "cmd_ctx->name" to function "__coverity_strcmp(char const *, char const *)", which dereferences it.
780    if (alias != NULL && strcmp(cmd_ctx->name, alias) != 0) {
781        cmd_ctx->alias = talloc_strdup(cmd_ctx, alias);
782        if (!cmd_ctx->alias) return ENOMEM;
783    }
784
785    return EOK;
786}

Fields changed

coverity: => 11647

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.10.1

Coverity bugs don't need a clone

rhbz: => 0

Moving tickets that didn't make 1.10.1 to the 1.10.2 bucket.

Moving tickets that didn't make 1.10.1 to 1.10.2

milestone: SSSD 1.10.1 => SSSD 1.10.2

Fields changed

owner: jcholast => lslebodn

Fields changed

patch: 0 => 1

Fields changed

status: new => assigned

resolution: => fixed
status: assigned => closed

Metadata Update from @jhrozek:
- Issue assigned to lslebodn
- Issue set to the milestone: SSSD 1.10.2

2 years ago

Login to comment on this ticket.

Metadata