#1970 Dereference after a NULL check in sshsrv_cmd.c
Closed: Fixed None Opened 10 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

7 years ago

SSSD is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in SSSD's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/SSSD/sssd/issues/3012

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata