From 472f7ea9367d7fa6968279d31b5ff0d465f99f83 Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Mon, 5 Nov 2018 12:29:06 +0100 Subject: [PATCH 1/3] fix [winpr/util]: memory leak in TestCmdLine --- winpr/libwinpr/utils/test/TestCmdLine.c | 39 ++++++++++++++++--------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/winpr/libwinpr/utils/test/TestCmdLine.c b/winpr/libwinpr/utils/test/TestCmdLine.c index dc85f6b62..c85303899 100644 --- a/winpr/libwinpr/utils/test/TestCmdLine.c +++ b/winpr/libwinpr/utils/test/TestCmdLine.c @@ -60,6 +60,7 @@ static COMMAND_LINE_ARGUMENT_A args[] = int TestCmdLine(int argc, char* argv[]) { int status; + int ret = -1; DWORD flags; long width = 0; long height = 0; @@ -70,12 +71,19 @@ int TestCmdLine(int argc, char* argv[]) flags = COMMAND_LINE_SIGIL_SLASH | COMMAND_LINE_SEPARATOR_COLON | COMMAND_LINE_SIGIL_PLUS_MINUS; testArgc = string_list_length(testArgv); command_line = string_list_copy(testArgv); + + if (!command_line) + { + printf("Argument duplication failed (not enough memory?)\n"); + return ret; + } + status = CommandLineParseArgumentsA(testArgc, command_line, args, flags, NULL, NULL, NULL); if (status != 0) { printf("CommandLineParseArgumentsA failure: %d\n", status); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "w"); @@ -83,7 +91,7 @@ int TestCmdLine(int argc, char* argv[]) if (strcmp("1024", arg->Value) != 0) { printf("CommandLineFindArgumentA: unexpected %s value %s\n", arg->Name, arg->Value); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "h"); @@ -91,7 +99,7 @@ int TestCmdLine(int argc, char* argv[]) if (strcmp("768", arg->Value) != 0) { printf("CommandLineFindArgumentA: unexpected %s value %s\n", arg->Name, arg->Value); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "f"); @@ -99,7 +107,7 @@ int TestCmdLine(int argc, char* argv[]) if (arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "admin"); @@ -107,7 +115,7 @@ int TestCmdLine(int argc, char* argv[]) if (!arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "multimon"); @@ -115,7 +123,7 @@ int TestCmdLine(int argc, char* argv[]) if (!arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "v"); @@ -123,7 +131,7 @@ int TestCmdLine(int argc, char* argv[]) if (strcmp("localhost:3389", arg->Value) != 0) { printf("CommandLineFindArgumentA: unexpected %s value %s\n", arg->Name, arg->Value); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "fonts"); @@ -131,7 +139,7 @@ int TestCmdLine(int argc, char* argv[]) if (!arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "wallpaper"); @@ -139,7 +147,7 @@ int TestCmdLine(int argc, char* argv[]) if (arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = CommandLineFindArgumentA(args, "help"); @@ -147,7 +155,7 @@ int TestCmdLine(int argc, char* argv[]) if (arg->Value) { printf("CommandLineFindArgumentA: unexpected %s value\n", arg->Name); - return -1; + goto out; } arg = args; @@ -168,14 +176,14 @@ int TestCmdLine(int argc, char* argv[]) width = strtol(arg->Value, NULL, 0); if (errno != 0) - return -1; + goto out; } CommandLineSwitchCase(arg, "h") { height = strtol(arg->Value, NULL, 0); if (errno != 0) - return -1; + goto out; } CommandLineSwitchDefault(arg) { @@ -187,8 +195,11 @@ int TestCmdLine(int argc, char* argv[]) if ((width != 1024) || (height != 768)) { printf("Unexpected width and height: Actual: (%ldx%ld), Expected: (1024x768)\n", width, height); - return -1; + goto out; } + ret = 0; - return 0; +out: + string_list_free(command_line); + return ret; } From 649404dd292f84ddcc05750d9fd71cb0683aff7a Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Mon, 5 Nov 2018 13:46:05 +0100 Subject: [PATCH 2/3] fix [libfreerdp/crypto]: memory leak in Test_x509_cert_info --- libfreerdp/crypto/test/Test_x509_cert_info.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libfreerdp/crypto/test/Test_x509_cert_info.c b/libfreerdp/crypto/test/Test_x509_cert_info.c index 4d3078753..4655f17ed 100644 --- a/libfreerdp/crypto/test/Test_x509_cert_info.c +++ b/libfreerdp/crypto/test/Test_x509_cert_info.c @@ -19,7 +19,7 @@ char* crypto_cert_subject_common_name_wo_length(X509* xcert) return crypto_cert_subject_common_name(xcert, & length); } -const char* certificate_path() +char* certificate_path() { /* Assume the .pem file is in the same directory as this source file. @@ -45,12 +45,12 @@ const char* certificate_path() else { /* No dirsep => relative path in same directory */ - return filename; + return _strdup(filename); } } const certificate_test_t certificate_tests[] = -{ + { { ENABLED, @@ -78,6 +78,7 @@ const certificate_test_t certificate_tests[] = "Certificate e-mail", crypto_cert_get_email, "testjean.testmartin@test.example.com" + }, { @@ -93,10 +94,10 @@ const certificate_test_t certificate_tests[] = crypto_cert_issuer, "CN = ADMINISTRATION CENTRALE DES TESTS, C = FR, O = MINISTERE DES TESTS, OU = 0002 110014016" }, - }; + int TestCertificateFile(const char* certificate_path, const certificate_test_t* certificate_tests, int count) { @@ -112,6 +113,7 @@ int TestCertificateFile(const char* certificate_path, const certificate_test_t* } certificate = PEM_read_X509(certificate_file, 0, 0, 0); + fclose(certificate_file); if (!certificate) { @@ -148,6 +150,7 @@ int TestCertificateFile(const char* certificate_path, const certificate_test_t* certificate_tests[i].expected_result); success = -1; } + free(result); } else { @@ -157,13 +160,16 @@ int TestCertificateFile(const char* certificate_path, const certificate_test_t* } fail: - fclose(certificate_file); + X509_free(certificate); return success; } int Test_x509_cert_info(int argc, char* argv[]) { - return TestCertificateFile(certificate_path(), certificate_tests, ARRAYSIZE(certificate_tests)); + char* cert_path = certificate_path(); + int ret = TestCertificateFile(cert_path, certificate_tests, ARRAYSIZE(certificate_tests)); + free(cert_path); + return ret; } From 37f818173521602428b4f815ca4145f8f42ae1b5 Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Mon, 5 Nov 2018 15:43:12 +0100 Subject: [PATCH 3/3] fix [libfreerdp/codec]: heap buffer overflow in TestFreeRDPCodecClear The examples and the allocated buffers assume that the destination starts with x 0 and y 0 and not 1/1. --- libfreerdp/codec/test/TestFreeRDPCodecClear.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libfreerdp/codec/test/TestFreeRDPCodecClear.c b/libfreerdp/codec/test/TestFreeRDPCodecClear.c index 2ae355ea0..bccec0cb5 100644 --- a/libfreerdp/codec/test/TestFreeRDPCodecClear.c +++ b/libfreerdp/codec/test/TestFreeRDPCodecClear.c @@ -58,7 +58,7 @@ static BOOL test_ClearDecompressExample(UINT32 nr, UINT32 width, UINT32 height, goto fail; status = clear_decompress(clear, pSrcData, SrcSize, width, height, - pDstData, PIXEL_FORMAT_XRGB32, 0, 1, 1, width, height, + pDstData, PIXEL_FORMAT_XRGB32, 0, 0, 0, width, height, NULL); printf("clear_decompress example %"PRIu32" status: %d\n", nr, status); fflush(stdout);