Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix issues found by static analyzers #125

Merged
merged 15 commits into from
Jul 30, 2024
Merged

fix issues found by static analyzers #125

merged 15 commits into from
Jul 30, 2024

Conversation

lnykryn
Copy link
Member

@lnykryn lnykryn commented May 6, 2024

No description provided.

@jamacku jamacku added the bugfix label May 6, 2024
@lnykryn lnykryn force-pushed the sast branch 6 times, most recently from 9cff630 to 3edcf07 Compare May 13, 2024 12:11
lnykryn added 15 commits May 13, 2024 14:32
@lnykryn
leveldb: fix memory leak
79fb75c 
We should unmap the memory before returning from the function:

Error: RESOURCE_LEAK (CWE-772):
chkconfig-1.26/leveldb.c:519: alloc_fn: Storage is returned from allocation function "mmap".
chkconfig-1.26/leveldb.c:519: var_assign: Assigning: "bufstart" = storage returned from "mmap(NULL, sb.st_size, 1, 1, fd, 0L)".
chkconfig-1.26/leveldb.c:528: leaked_storage: Variable "bufstart" going out of scope leaks the storage it points to.
  526|       if (tmpbufstart == NULL) {
  527|           close(fd);
  528|->         return -1;
  529|       }
  530|
@lnykryn
chkconfig: fix leak
75242a3 
Free the services field after we printed its contents.
Also remove the check for the return value of showServiceInfo,
since it is called with "forgiving" flag, so it can't return
an error.

Error: RESOURCE_LEAK (CWE-772):
chkconfig-1.26/chkconfig.c:524: alloc_arg: "readServices" allocates memory that is stored into "services".
chkconfig-1.26/chkconfig.c:528: noescape: Resource "services" is not freed or pointed-to in "qsort".
chkconfig-1.26/chkconfig.c:547: leaked_storage: Variable "services" going out of scope leaks the storage it points to.
  545|               fprintf(stderr, _("failed to open directory %s: %s\n"), XINETDDIR,
  546|                       strerror(err));
  547|->             return 1;
  548|           }
  549|           numServices = 0;
@lnykryn
leveldb.c: fix memory leak
5539345 
Error: RESOURCE_LEAK (CWE-772):
chkconfig-1.26/leveldb.c:1099: alloc_arg: "readSystemdUnitProperty" allocates memory that is stored into "t".
chkconfig-1.26/leveldb.c:1103: noescape: Resource "t" is not freed or pointed-to in "strcmp".
chkconfig-1.26/leveldb.c:1103: noescape: Resource "t" is not freed or pointed-to in "strcmp".
chkconfig-1.26/leveldb.c:1136: leaked_storage: Variable "t" going out of scope leaks the storage it points to.
 1134|           free(ret);
 1135|       }
 1136|->     return r;
 1137|   }
 1138|
@lnykryn
leveldb: fix memory leak
1d7a9eb 
Error: RESOURCE_LEAK (CWE-772):
chkconfig-1.26/leveldb.c:142: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/leveldb.c:142: var_assign: Assigning: "desc" = storage returned from "malloc(end - start + 1L)".
chkconfig-1.26/leveldb.c:143: noescape: Resource "desc" is not freed or pointed-to in "strncpy". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/leveldb.c:148: noescape: Resource "desc" is not freed or pointed-to in "strlen".
chkconfig-1.26/leveldb.c:149: noescape: Resource "desc" is not freed or pointed-to in "strlen".
chkconfig-1.26/leveldb.c:155: leaked_storage: Variable "desc" going out of scope leaks the storage it points to.
  153|                   start++;
  154|               if (start == bufstop || *start != '#') {
  155|->                 return 1;
  156|               }
  157|
@lnykryn
leveldb: add destructors for service
0b1f474
@lnykryn
chkconfig: fix memory leak when deleting a service
0769673 
Error: RESOURCE_LEAK (CWE-772):
chkconfig-1.26/chkconfig.c:125: alloc_arg: "readServices" allocates memory that is stored into "services".
chkconfig-1.26/chkconfig.c:163: leaked_storage: Variable "services" going out of scope leaks the storage it points to.
  161|           }
  162|       }
  163|->     return 0;
  164|   }
  165|
@lnykryn
chkconfig: fix leak
d855467 
Error: RESOURCE_LEAK (CWE-772): [#def23] [important]
chkconfig-1.26/chkconfig.c:308:5: alloc_arg: "readServices" allocates memory that is stored into "servs".
chkconfig-1.26/chkconfig.c:330:5: noescape: Resource "servs" is not freed or pointed-to in "frobOneDependencies".
chkconfig-1.26/chkconfig.c:331:9: leaked_storage: Variable "servs" going out of scope leaks the storage it points to.
  329|       /* Resolve our target */
  330|       if (frobOneDependencies(s, servs, numservs, 1, LSB) == -1)
  331|->         return 1;
  332|       return 0;
  333|   }
@lnykryn
chkconfig: fix leak
587e892 
Error: RESOURCE_LEAK (CWE-772): [#def25] [important]
chkconfig-1.26/chkconfig.c:590:13: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/chkconfig.c:590:13: var_assign: Assigning: "tmp" = storage returned from "malloc(strlen(s->name) + 5UL)".
chkconfig-1.26/chkconfig.c:591:13: noescape: Resource "tmp" is not freed or pointed-to in "sprintf". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/chkconfig.c:592:13: noescape: Resource "tmp" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/chkconfig.c:593:9: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to.
  591|               sprintf(tmp, "%s:", s->name);
  592|               printf("\t%-15s\t%s\n", tmp, s->levels ? _("on") : _("off"));
  593|->         }
  594|           closedir(dir);
  595|           free(t);
@lnykryn
leveldb: fix leak
195e6f0 
Error: RESOURCE_LEAK (CWE-772): [#def24] [important]
chkconfig-1.26/leveldb.c:230:5: alloc_arg: "asprintf" allocates memory that is stored into "filename". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/leveldb.c:232:5: noescape: Resource "filename" is not freed or pointed-to in "open". [Note: The source code implementation of the function has been overridden by a user model.]
chkconfig-1.26/leveldb.c:307:5: leaked_storage: Variable "filename" going out of scope leaks the storage it points to.
  305|       }
  306|       *service = serv;
  307|->     return 0;
  308|   out_err:
  309|       if (fd >= 0)
@lnykryn
leveldb: fix leak
1a9b859 
Error: RESOURCE_LEAK (CWE-772): [#def25] [important]
chkconfig-1.26/leveldb.c:237:5: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/leveldb.c:237:5: var_assign: Assigning: "buf" = storage returned from "malloc(sb.st_size + 1L)".
chkconfig-1.26/leveldb.c:238:5: noescape: Resource "buf" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/leveldb.c:243:5: var_assign: Assigning: "start" = "buf".
chkconfig-1.26/leveldb.c:245:9: noescape: Resource "buf" is not freed or pointed-to in "strchr".
chkconfig-1.26/leveldb.c:250:13: noescape: Resource "buf" is not freed or pointed-to in "strncmp".
chkconfig-1.26/leveldb.c:254:17: noescape: Resource "buf + 9" is not freed or pointed-to in "strncmp".
chkconfig-1.26/leveldb.c:307:5: leaked_storage: Variable "start" going out of scope leaks the storage it points to.
  305|       }
  306|       *service = serv;
  307|->     return 0;
  308|   out_err:
  309|       if (fd >= 0)
@lnykryn
leveldb: fix leak
f310aa0 
Error: RESOURCE_LEAK (CWE-772): [#def25] [important]
chkconfig-1.26/leveldb.c:352:5: alloc_fn: Storage is returned from allocation function "opendir".
chkconfig-1.26/leveldb.c:352:5: var_assign: Assigning: "dir" = storage returned from "opendir("/etc/init.d")".
chkconfig-1.26/leveldb.c:358:5: noescape: Resource "dir" is not freed or pointed-to in "readdir".
chkconfig-1.26/leveldb.c:386:5: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
  384|       }
  385|       *services = servs;
  386|->     return numservs;
  387|   }
  388|
@lnykryn
leveldb: fix leak
0836dd1 
Error: RESOURCE_LEAK (CWE-772): [#def26] [important]
chkconfig-1.26/leveldb.c:894:5: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/leveldb.c:894:5: var_assign: Assigning: "buf" = storage returned from "malloc(sb.st_size + 1L)".
chkconfig-1.26/leveldb.c:895:5: noescape: Resource "buf" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.]
chkconfig-1.26/leveldb.c:911:9: var_assign: Assigning: "tmp" = "buf".
chkconfig-1.26/leveldb.c:912:9: noescape: Resource "buf" is not freed or pointed-to in "strchr".
chkconfig-1.26/leveldb.c:919:9: noescape: Resource "buf" is not freed or pointed-to in "strncmp".
chkconfig-1.26/leveldb.c:919:9: noescape: Resource "buf" is not freed or pointed-to in "strlen".
chkconfig-1.26/leveldb.c:920:13: noescape: Resource "tmp" is not freed or pointed-to in "strlen".
chkconfig-1.26/leveldb.c:920:13: noescape: Resource "tmp" is not freed or pointed-to in "write".
chkconfig-1.26/leveldb.c:936:5: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to.
  934|           fprintf(stderr, _("Unable to set SELinux context for %s: %s\n"),
  935|                   oldfname, strerror(errno));
  936|->     return (r);
  937|   }
  938|
@lnykryn
leveldb: fix leak
5c57fff 
Error: CPPCHECK_WARNING (CWE-401): [#def31] [important]
chkconfig-1.26/leveldb.c:757: error[memleak]: Memory leak: serv.softStopDeps
  755|           ((serv.levels == -1) || !serv.desc ||
  756|            (!serv.isLSB && (serv.sPriority == -1 || serv.kPriority == 100)))) {
  757|->         return 1;
  758|       }
  759|

Error: CPPCHECK_WARNING (CWE-401): [#def32] [important]
chkconfig-1.26/leveldb.c:757: error[memleak]: Memory leak: serv.startDeps
  755|           ((serv.levels == -1) || !serv.desc ||
  756|            (!serv.isLSB && (serv.sPriority == -1 || serv.kPriority == 100)))) {
  757|->         return 1;
  758|       }
  759|

Error: CPPCHECK_WARNING (CWE-401): [#def33] [important]
chkconfig-1.26/leveldb.c:757: error[memleak]: Memory leak: serv.stopDeps
  755|           ((serv.levels == -1) || !serv.desc ||
  756|            (!serv.isLSB && (serv.sPriority == -1 || serv.kPriority == 100)))) {
  757|->         return 1;
  758|       }
  759|
@lnykryn
leveldb: security_context_t is deprecated
fb0660d 
SELinuxProject/selinux@7a124ca
@lnykryn
ntsysv: fix leaks
0a3cd94 
Error: RESOURCE_LEAK (CWE-772): [#def1] [important]
chkconfig-1.26/ntsysv.c:316:5: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/ntsysv.c:316:5: var_assign: Assigning: "services" = storage returned from "malloc(88UL * numServicesAlloced)".
chkconfig-1.26/ntsysv.c:321:9: leaked_storage: Variable "services" going out of scope leaks the storage it points to.
  319|           fprintf(stderr, "failed to open " RUNLEVELS "/init.d: %s\n",
  320|                   strerror(errno));
  321|->         return 2;
  322|       }
  323|

Error: RESOURCE_LEAK (CWE-772): [#def2] [important]
chkconfig-1.26/ntsysv.c:316:5: alloc_fn: Storage is returned from allocation function "malloc".
chkconfig-1.26/ntsysv.c:316:5: var_assign: Assigning: "services" = storage returned from "malloc(88UL * numServicesAlloced)".
chkconfig-1.26/ntsysv.c:346:9: noescape: Resource "services + numServices" is not freed or pointed-to in "readServiceInfo".
chkconfig-1.26/ntsysv.c:375:13: leaked_storage: Variable "services" going out of scope leaks the storage it points to.
  373|               fprintf(stderr, "failed to open " XINETDDIR ": %s\n",
  374|                       strerror(errno));
  375|->             return 2;
  376|           }
  377|

Error: RESOURCE_LEAK (CWE-772): [#def3] [important]
chkconfig-1.26/ntsysv.c:393:17: alloc_fn: Storage is returned from allocation function "realloc".
chkconfig-1.26/ntsysv.c:393:17: var_assign: Assigning: "services" = storage returned from "realloc(services, numServicesAlloced * 88UL)".
chkconfig-1.26/ntsysv.c:397:13: noescape: Resource "services + numServices" is not freed or pointed-to in "readXinetdServiceInfo".
chkconfig-1.26/ntsysv.c:403:17: leaked_storage: Variable "services" going out of scope leaks the storage it points to.
  401|                           ent->d_name, strerror(errno));
  402|                   closedir(dir);
  403|->                 return 2;
  404|               } else if (!rc)
  405|                   numServices++;

Error: RESOURCE_LEAK (CWE-772): [#def4] [important]
chkconfig-1.26/ntsysv.c:372:9: alloc_fn: Storage is returned from allocation function "opendir".
chkconfig-1.26/ntsysv.c:372:9: var_assign: Assigning: "dir" = storage returned from "opendir("/etc/xinetd.d")".
chkconfig-1.26/ntsysv.c:378:9: noescape: Resource "dir" is not freed or pointed-to in "readdir".
chkconfig-1.26/ntsysv.c:422:5: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
  420|       *numServicesPtr = numServices;
  421|
  422|->     return 0;
  423|   }
  424|

Error: CPPCHECK_WARNING (CWE-401): [#def5] [important]
chkconfig-1.26/ntsysv.c:321: error[memleak]: Memory leak: services
  319|           fprintf(stderr, "failed to open " RUNLEVELS "/init.d: %s\n",
  320|                   strerror(errno));
  321|->         return 2;
  322|       }
  323|
@jamacku

This comment was marked as outdated.

Sign in to view
jamacku
jamacku approved these changes Jul 29, 2024
View reviewed changes
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes. I have tried to test chkconfig manually, and it seems to be working fine.

LGTM

@lnykryn lnykryn marked this pull request as ready for review July 30, 2024 11:25
@jamacku jamacku merged commit c03280c into fedora-sysv:main Jul 30, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamacku jamacku jamacku approved these changes

Assignees
No one assigned
Labels
bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lnykryn @jamacku