From e9cf8a977abb02e04511544df42e97cffe6976b5 Mon Sep 17 00:00:00 2001 From: Piotr Sawicki Date: Thu, 6 Apr 2017 13:24:17 +0200 Subject: [PATCH] Fix issues detected by SVACE * Check the capacity of 'alias' array before calling strncpy() * Fix a memory leak in CreateFromFile() * Fix a leak of file descriptors (returned by accept()) Change-Id: I89c55178510c6f23649fd54c65cb1d17ce278398 --- src/server/include/cert-server-logic.h | 4 ++-- src/server/src/cert-server-logic.c | 12 +++++++++--- src/server/src/cert-server-main.c | 10 +++++++--- src/vcore/Certificate.cpp | 10 +++++----- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/server/include/cert-server-logic.h b/src/server/include/cert-server-logic.h index 68e3ab9..3e42f10 100644 --- a/src/server/include/cert-server-logic.h +++ b/src/server/include/cert-server-logic.h @@ -1,5 +1,5 @@ /** - * Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2016 - 2017 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -52,7 +52,7 @@ int getCertificateListFromStore(int reqType, CertStoreType storeType, char **ppCertListBuffer, size_t *bufferLen, size_t *certCount); int getCertificateAliasFromStore(CertStoreType storeType, const char *gname, - char *alias); + char *alias, size_t aliasSize); int loadCertificatesFromStore(CertStoreType storeType, const char *gname, char **ppCertBlockBuffer, diff --git a/src/server/src/cert-server-logic.c b/src/server/src/cert-server-logic.c index 5374585..45b2942 100644 --- a/src/server/src/cert-server-logic.c +++ b/src/server/src/cert-server-logic.c @@ -1,5 +1,5 @@ /** - * Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2016 - 2017 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1461,7 +1461,7 @@ error: } int getCertificateAliasFromStore(CertStoreType storeType, const char *gname, - char *alias) + char *alias, size_t aliasSize) { int result = CERTSVC_SUCCESS; int records = 0; @@ -1492,7 +1492,13 @@ int getCertificateAliasFromStore(CertStoreType storeType, const char *gname, goto error; } - strncpy(alias, text, strlen(text)); + if (strlen(text) >= aliasSize) { + SLOGE("Unable to get the alias name for the gname passed, common_name too long."); + result = CERTSVC_FAIL; + goto error; + } + + strncpy(alias, text, aliasSize); if (strlen(alias) == 0) { SLOGE("Unable to get the alias name for the gname passed."); diff --git a/src/server/src/cert-server-main.c b/src/server/src/cert-server-main.c index d8bdd56..072aa49 100644 --- a/src/server/src/cert-server-main.c +++ b/src/server/src/cert-server-main.c @@ -1,5 +1,5 @@ /** - * Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2016 - 2017 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -67,7 +67,7 @@ int CertSvcGetSocketFromSystemd(int *pSockfd) void CertSvcServerComm(void) { int server_sockfd = 0; - int client_sockfd = 0; + int client_sockfd = -1; int read_len = 0; int client_len = 0; struct sockaddr_un clientaddr; @@ -296,7 +296,8 @@ void CertSvcServerComm(void) send_data.result = getCertificateAliasFromStore( recv_data.storeType, recv_data.gname, - send_data.common_name); + send_data.common_name, + sizeof(send_data.common_name)); result = send(client_sockfd, (char *)&send_data, sizeof(send_data), MSG_NOSIGNAL); break; @@ -325,6 +326,9 @@ void CertSvcServerComm(void) if (result <= 0) SLOGE("send failed :%d, errno %d try once", result, errno); + + close(client_sockfd); + client_sockfd = -1; } Error_close_exit: diff --git a/src/vcore/Certificate.cpp b/src/vcore/Certificate.cpp index 86b0d08..875adca 100644 --- a/src/vcore/Certificate.cpp +++ b/src/vcore/Certificate.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2016 - 2017 Samsung Electronics Co., Ltd All Rights Reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -136,20 +136,20 @@ CertificatePtr Certificate::createFromFile(const std::string &location) VcoreThrowMsg(Certificate::Exception::WrongParamError, "File content is empty : " << location); - unsigned char *content = new unsigned char[filesize + 1]; + std::unique_ptr content(new unsigned char[filesize + 1]); if (content == NULL) VcoreThrowMsg(Certificate::Exception::InternalError, "Fail to allocate memory."); - memset(content, 0x00, filesize + 1); + memset(content.get(), 0x00, filesize + 1); rewind(fp); - if (fread(content, sizeof(unsigned char), filesize, fp) != static_cast(filesize)) + if (fread(content.get(), sizeof(unsigned char), filesize, fp) != static_cast(filesize)) VcoreThrowMsg(Certificate::Exception::InternalError, "file read failed. wrong size : " << location); content[filesize] = '\0'; - const unsigned char *ptr = reinterpret_cast(content); + const unsigned char *ptr = reinterpret_cast(content.get()); x509 = d2i_X509(NULL, &ptr, filesize); if (x509 == NULL) -- 2.7.4