URL: https://github.com/SSSD/sssd/pull/5902
Author: alexey-tikhonov
 Title: #5902: SSS_CLIENT: fixed few covscan issues
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5902/head:pr5902
git checkout pr5902
From dc56b8e253b7fb0d0ada123715bdb4e321c53063 Mon Sep 17 00:00:00 2001
From: Alexey Tikhonov <atikh...@redhat.com>
Date: Wed, 1 Dec 2021 21:56:53 +0100
Subject: [PATCH] SSS_CLIENT: fixed few covscan issues

Fixes following covscan issues:
```
Error: TAINTED_SCALAR (CWE-20):
sssd-2.6.1/src/sss_client/subid/sss_subid.c:75: tainted_argument: Calling function "sss_cli_make_request_with_checks" taints argument "*repbuf".
sssd-2.6.1/src/sss_client/subid/sss_subid.c:94: identity_transfer: Passing "repbuf + 4UL" as argument 2 to function "safealign_memcpy", which sets "num_results" to the dereference of that argument.
sssd-2.6.1/src/sss_client/subid/sss_subid.c:94: tainted_data_transitive: Call to function "safealign_memcpy" with tainted argument "*repbuf" transitively taints "num_results".
sssd-2.6.1/src/sss_client/subid/sss_subid.c:116: tainted_data: Passing tainted expression "num_results * 16UL" to "malloc", which uses it as an allocation size.
sssd-2.6.1/src/sss_client/subid/sss_subid.c:116: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
 #  114|       }
 #  115|
 #  116|->     *ranges = malloc(num_results * sizeof(struct subid_range));
 #  117|       if (!*ranges) {
 #  118|           free(repbuf);

Error: TAINTED_SCALAR (CWE-20):
sssd-2.6.1/src/sss_client/subid/sss_subid.c:75: tainted_argument: Calling function "sss_cli_make_request_with_checks" taints argument "*repbuf".
sssd-2.6.1/src/sss_client/subid/sss_subid.c:94: identity_transfer: Passing "repbuf + 4UL" as argument 2 to function "safealign_memcpy", which sets "num_results" to the dereference of that argument.
sssd-2.6.1/src/sss_client/subid/sss_subid.c:94: tainted_data_transitive: Call to function "safealign_memcpy" with tainted argument "*repbuf" transitively taints "num_results".
sssd-2.6.1/src/sss_client/subid/sss_subid.c:122: tainted_data: Using tainted variable "num_results" as a loop boundary.
sssd-2.6.1/src/sss_client/subid/sss_subid.c:122: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
 #  120|       }
 #  121|
 #  122|->     for (uint32_t c = 0; c < num_results; ++c) {
 #  123|           SAFEALIGN_COPY_UINT32(&val, repbuf + index, &index);
 #  124|           (*ranges)[c].start = val;

Error: TAINTED_SCALAR (CWE-20):
sssd-2.6.1/src/sss_client/subid/sss_subid.c:176: tainted_argument: Calling function "shadow_subid_list_owner_ranges" taints argument "amount".
sssd-2.6.1/src/sss_client/subid/sss_subid.c:183: tainted_data: Using tainted variable "amount" as a loop boundary.
sssd-2.6.1/src/sss_client/subid/sss_subid.c:183: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
 #  181|       *result = false;
 #  182|
 #  183|->     for (int i = 0; i < amount; ++i) {
 #  184|           if ((range[i].start <= start) &&
 #  185|               (range[i].start + range[i].count >= end)) {
```

Resolves: https://github.com/SSSD/sssd/issues/5878
---
 src/sss_client/subid/sss_subid.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/sss_client/subid/sss_subid.c b/src/sss_client/subid/sss_subid.c
index ae74ece3cd..f1fbe34ee3 100644
--- a/src/sss_client/subid/sss_subid.c
+++ b/src/sss_client/subid/sss_subid.c
@@ -78,7 +78,11 @@ enum subid_status shadow_subid_list_owner_ranges(const char *user,
                                            SSS_NSS_SOCKET_NAME);
     sss_nss_unlock();
 
-    if ((ret != SSS_STATUS_SUCCESS) || (errnop != EOK)) {
+    if ( (ret != SSS_STATUS_SUCCESS) || (errnop != EOK)
+        /* response must contain at least the "payload header" */
+        || (replen < 2*sizeof(uint32_t))
+        /* and even number of 'uint32_t' */
+        || (replen % (2*sizeof(uint32_t)) != 0) ) {
         free(repbuf);
         if (ret == SSS_STATUS_UNAVAIL) {
             return SUBID_STATUS_ERROR_CONN;
@@ -87,11 +91,20 @@ enum subid_status shadow_subid_list_owner_ranges(const char *user,
     }
 
     SAFEALIGN_COPY_UINT32(&num_results, repbuf, NULL);
+    if (num_results > (replen/sizeof(uint32_t) - 2)/2) {
+        free(repbuf);
+        return SUBID_STATUS_ERROR;
+    }
+
     if (id_type == ID_TYPE_UID) {
         index = 2 * sizeof(uint32_t);
     } else {
         index = (2 + 2*num_results) * sizeof(uint32_t);
         SAFEALIGN_COPY_UINT32(&num_results, repbuf + sizeof(uint32_t), NULL);
+        if (num_results > ((replen - index)/sizeof(uint32_t)/2)) {
+            free(repbuf);
+            return SUBID_STATUS_ERROR;
+        }
     }
     if (num_results == 0) {
         /* TODO: how to distinguish "user not found" vs "user doesn't have ranges defined" here?
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to