From 1cee185e3cbcee973a8059272bf6348004eeb1e1 Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Fri, 26 Jun 2015 15:58:01 +0200 Subject: [PATCH] hardening: check fread and fwrite return values --- channels/printer/client/printer_cups.c | 2 +- client/common/file.c | 19 +- libfreerdp/core/certificate.c | 14 +- libfreerdp/crypto/test/TestKnownHosts.c | 16 +- libfreerdp/locale/timezone.c | 7 +- libfreerdp/utils/pcap.c | 18 +- server/shadow/Win/win_wds.c | 23 ++- winpr/libwinpr/sspi/test/TestSchannel.c | 30 +-- winpr/libwinpr/utils/image.c | 49 +++-- winpr/libwinpr/utils/ini.c | 42 ++-- winpr/libwinpr/utils/lodepng/lodepng.c | 8 +- winpr/libwinpr/utils/wlog/BinaryAppender.c | 6 +- winpr/libwinpr/utils/wlog/DataMessage.c | 6 +- winpr/libwinpr/utils/wlog/PacketMessage.c | 214 ++++++++++++--------- winpr/tools/makecert/makecert.c | 149 +++++++------- 15 files changed, 336 insertions(+), 267 deletions(-) diff --git a/channels/printer/client/printer_cups.c b/channels/printer/client/printer_cups.c index 0d6fcbeb9..715bb9be5 100644 --- a/channels/printer/client/printer_cups.c +++ b/channels/printer/client/printer_cups.c @@ -91,7 +91,7 @@ static void printer_cups_write_printjob(rdpPrintJob* printjob, BYTE* data, int s if (fwrite(data, 1, size, fp) < size) { - + // FIXME once this function doesn't return void anymore! } fclose(fp); diff --git a/client/common/file.c b/client/common/file.c index 5da86c9ea..f4b23fa02 100644 --- a/client/common/file.c +++ b/client/common/file.c @@ -800,17 +800,28 @@ BOOL freerdp_client_write_rdp_file(const rdpFile* file, const char* name, BOOL u ConvertToUnicode(CP_UTF8, 0, buffer, length, &unicodestr, 0); /* Write multi-byte header */ - fwrite(BOM_UTF16_LE, sizeof(BYTE), 2, fp); - fwrite(unicodestr, 2, length, fp); + if (fwrite(BOM_UTF16_LE, sizeof(BYTE), 2, fp) != 2 || + fwrite(unicodestr, 2, length, fp) != length) + { + free(buffer); + free(unicodestr); + fclose(fp); + return FALSE; + } free(unicodestr); } else { - fwrite(buffer, 1, length, fp); + if (fwrite(buffer, 1, length, fp) != length) + { + free(buffer); + fclose(fp); + return FALSE; + } } - status = fflush(fp); + fflush(fp); status = fclose(fp); } diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index d876d5d73..3fde935f5 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -684,16 +684,20 @@ rdpRsaKey* key_new(const char* keyfile) goto out_free; } - fseek(fp, 0, SEEK_END); - length = ftell(fp); - fseek(fp, 0, SEEK_SET); + if (fseek(fp, 0, SEEK_END) < 0) + goto out_free; + if ((length = ftell(fp)) < 0) + goto out_free; + if (fseek(fp, 0, SEEK_SET) < 0) + goto out_free; buffer = (BYTE*) malloc(length); if (!buffer) goto out_free; - fread((void*) buffer, length, 1, fp); + if (fread((void*) buffer, length, 1, fp) != 1) + goto out_free; fclose(fp); fp = NULL; @@ -703,7 +707,6 @@ rdpRsaKey* key_new(const char* keyfile) goto out_free; rsa = PEM_read_bio_RSAPrivateKey(bio, NULL, NULL, NULL); - BIO_free(bio); free(buffer); buffer = NULL; @@ -756,6 +759,7 @@ rdpRsaKey* key_new(const char* keyfile) crypto_reverse(key->exponent, sizeof(key->exponent)); RSA_free(rsa); return key; + out_free_modulus: free(key->Modulus); out_free_rsa: diff --git a/libfreerdp/crypto/test/TestKnownHosts.c b/libfreerdp/crypto/test/TestKnownHosts.c index 0adf02ddf..534cc90cd 100644 --- a/libfreerdp/crypto/test/TestKnownHosts.c +++ b/libfreerdp/crypto/test/TestKnownHosts.c @@ -46,8 +46,9 @@ static int prepare(const char* currentFileV2, const char* legacyFileV2, const ch for (i=0; ifp) == 1; } -BOOL pcap_write_header(rdpPcap* pcap, pcap_header* header) +static BOOL pcap_write_header(rdpPcap* pcap, pcap_header* header) { return fwrite((void*) header, sizeof(pcap_header), 1, pcap->fp) == 1; } -BOOL pcap_read_record_header(rdpPcap* pcap, pcap_record_header* record) +static BOOL pcap_read_record_header(rdpPcap* pcap, pcap_record_header* record) { return fread((void*) record, sizeof(pcap_record_header), 1, pcap->fp) == 1; } -void pcap_write_record_header(rdpPcap* pcap, pcap_record_header* record) +static BOOL pcap_write_record_header(rdpPcap* pcap, pcap_record_header* record) { - fwrite((void*) record, sizeof(pcap_record_header), 1, pcap->fp); + return fwrite((void*) record, sizeof(pcap_record_header), 1, pcap->fp) == 1; } -BOOL pcap_read_record(rdpPcap* pcap, pcap_record* record) +static BOOL pcap_read_record(rdpPcap* pcap, pcap_record* record) { if (!pcap_read_record_header(pcap, &record->header)) return FALSE; @@ -91,10 +91,10 @@ BOOL pcap_read_record(rdpPcap* pcap, pcap_record* record) return TRUE; } -void pcap_write_record(rdpPcap* pcap, pcap_record* record) +static BOOL pcap_write_record(rdpPcap* pcap, pcap_record* record) { - pcap_write_record_header(pcap, &record->header); - fwrite(record->data, record->length, 1, pcap->fp); + return pcap_write_record_header(pcap, &record->header) && + (fwrite(record->data, record->length, 1, pcap->fp) == 1); } BOOL pcap_add_record(rdpPcap* pcap, void* data, UINT32 length) diff --git a/server/shadow/Win/win_wds.c b/server/shadow/Win/win_wds.c index 1ec82172f..d076d3059 100644 --- a/server/shadow/Win/win_wds.c +++ b/server/shadow/Win/win_wds.c @@ -764,21 +764,24 @@ int win_shadow_wds_init(winShadowSubsystem* subsystem) WLog_INFO(TAG, "ConnectionString: %s", file->ConnectionString2); - if (0) +#if 0 + FILE* fp; + size_t size; + + fp = fopen("inv.xml", "w+b"); + + if (fp) { - FILE* fp; - size_t size; - - fp = fopen("inv.xml", "w+b"); - - if (fp) + size = strlen(file->ConnectionString2); + if (fwrite(file->ConnectionString2, 1, size, fp) != size || fwrite("\r\n", 1, 2, fp) != 2) { - size = strlen(file->ConnectionString2); - fwrite(file->ConnectionString2, 1, size, fp); - fwrite("\r\n", 1, 2, fp); fclose(fp); + WLog_ERR(TAG, "Problem writing to inv.xml"); + return -1; } + fclose(fp); } +#endif status = win_shadow_rdp_init(subsystem); diff --git a/winpr/libwinpr/sspi/test/TestSchannel.c b/winpr/libwinpr/sspi/test/TestSchannel.c index 9a7b067c7..6ffbb70cc 100644 --- a/winpr/libwinpr/sspi/test/TestSchannel.c +++ b/winpr/libwinpr/sspi/test/TestSchannel.c @@ -549,34 +549,42 @@ static void* schannel_test_server_thread(void* arg) int dump_test_certificate_files() { FILE* fp; - char* fullpath; + char* fullpath = NULL; + int ret = -1; + /* * Output Certificate File */ fullpath = GetCombinedPath("/tmp", "localhost.crt"); - fp = fopen(fullpath, "w+"); + if (!fullpath) + return -1; + fp = fopen(fullpath, "w+"); if (fp) { - fwrite((void*) test_localhost_crt, sizeof(test_localhost_crt), 1, fp); + if (fwrite((void*) test_localhost_crt, sizeof(test_localhost_crt), 1, fp) != 1) + goto out_fail; fclose(fp); + fp = NULL; } - free(fullpath); + /* * Output Private Key File */ fullpath = GetCombinedPath("/tmp", "localhost.key"); + if (!fullpath) + return -1; fp = fopen(fullpath, "w+"); + if (fp && fwrite((void*) test_localhost_key, sizeof(test_localhost_key), 1, fp) != 1) + goto out_fail; - if (fp) - { - fwrite((void*) test_localhost_key, sizeof(test_localhost_key), 1, fp); - fclose(fp); - } - + ret = 1; +out_fail: free(fullpath); - return 1; + if (fp) + fclose(fp); + return ret; } int TestSchannel(int argc, char* argv[]) diff --git a/winpr/libwinpr/utils/image.c b/winpr/libwinpr/utils/image.c index d6c5a0f6e..abaa7bf77 100644 --- a/winpr/libwinpr/utils/image.c +++ b/winpr/libwinpr/utils/image.c @@ -39,6 +39,7 @@ int winpr_bitmap_write(const char* filename, BYTE* data, int width, int height, FILE* fp; WINPR_BITMAP_FILE_HEADER bf; WINPR_BITMAP_INFO_HEADER bi; + int ret = 1; fp = fopen(filename, "w+b"); @@ -67,13 +68,14 @@ int winpr_bitmap_write(const char* filename, BYTE* data, int width, int height, bi.biClrImportant = 0; bi.biSize = sizeof(WINPR_BITMAP_INFO_HEADER); - fwrite((void*) &bf, sizeof(WINPR_BITMAP_FILE_HEADER), 1, fp); - fwrite((void*) &bi, sizeof(WINPR_BITMAP_INFO_HEADER), 1, fp); - fwrite((void*) data, bi.biSizeImage, 1, fp); + if (fwrite((void*) &bf, sizeof(WINPR_BITMAP_FILE_HEADER), 1, fp) != 1 || + fwrite((void*) &bi, sizeof(WINPR_BITMAP_INFO_HEADER), 1, fp) != 1 || + fwrite((void*) data, bi.biSizeImage, 1, fp) != 1) + ret = -1; fclose(fp); - return 1; + return ret; } int winpr_image_write(wImage* image, const char* filename) @@ -113,9 +115,8 @@ int winpr_image_png_read_fp(wImage* image, FILE* fp) if (!data) return -1; - fread((void*) data, size, 1, fp); - - fclose(fp); + if (fread((void*) data, size, 1, fp) != 1) + return -1; lodepng_status = lodepng_decode32(&(image->data), &width, &height, data, size); @@ -163,14 +164,16 @@ int winpr_image_bitmap_read_fp(wImage* image, FILE* fp) WINPR_BITMAP_FILE_HEADER bf; WINPR_BITMAP_INFO_HEADER bi; - fread((void*) &bf, sizeof(WINPR_BITMAP_FILE_HEADER), 1, fp); + if (fread((void*) &bf, sizeof(WINPR_BITMAP_FILE_HEADER), 1, fp) != 1) + return -1; if ((bf.bfType[0] != 'B') || (bf.bfType[1] != 'M')) return -1; image->type = WINPR_IMAGE_BITMAP; - fread((void*) &bi, sizeof(WINPR_BITMAP_INFO_HEADER), 1, fp); + if (fread((void*) &bi, sizeof(WINPR_BITMAP_INFO_HEADER), 1, fp) != 1) + return -1; if (ftell(fp) != bf.bfOffBits) { @@ -201,7 +204,12 @@ int winpr_image_bitmap_read_fp(wImage* image, FILE* fp) if (!vFlip) { - fread((void*) image->data, bi.biSizeImage, 1, fp); + if (fread((void*) image->data, bi.biSizeImage, 1, fp) != 1) + { + free(image->data); + image->data = NULL; + return -1; + } } else { @@ -209,13 +217,16 @@ int winpr_image_bitmap_read_fp(wImage* image, FILE* fp) for (index = 0; index < image->height; index++) { - fread((void*) pDstData, image->scanline, 1, fp); + if (fread((void*) pDstData, image->scanline, 1, fp) != 1) + { + free(image->data); + image->data = NULL; + return -1; + } pDstData -= image->scanline; } } - fclose(fp); - return 1; } @@ -302,8 +313,11 @@ int winpr_image_read(wImage* image, const char* filename) return -1; } - fread((void*) &sig, sizeof(sig), 1, fp); - fseek(fp, 0, SEEK_SET); + if (fread((void*) &sig, sizeof(sig), 1, fp) != 1 || fseek(fp, 0, SEEK_SET) < 0) + { + fclose(fp); + return -1; + } if ((sig[0] == 'B') && (sig[1] == 'M')) { @@ -316,10 +330,7 @@ int winpr_image_read(wImage* image, const char* filename) image->type = WINPR_IMAGE_PNG; status = winpr_image_png_read_fp(image, fp); } - else - { - fclose(fp); - } + fclose(fp); return status; } diff --git a/winpr/libwinpr/utils/ini.c b/winpr/libwinpr/utils/ini.c index 4ffa191f2..09c0ee74c 100644 --- a/winpr/libwinpr/utils/ini.c +++ b/winpr/libwinpr/utils/ini.c @@ -77,38 +77,28 @@ int IniFile_Load_File(wIniFile* ini, const char* filename) if (IniFile_Open_File(ini, filename) < 0) return -1; - fseek(ini->fp, 0, SEEK_END); + if (fseek(ini->fp, 0, SEEK_END) < 0) + goto out_file; fileSize = ftell(ini->fp); - fseek(ini->fp, 0, SEEK_SET); + if (fileSize < 0) + goto out_file; + if (fseek(ini->fp, 0, SEEK_SET) < 0) + goto out_file; ini->line = NULL; ini->nextLine = NULL; ini->buffer = NULL; if (fileSize < 1) - { - fclose(ini->fp); - ini->fp = NULL; - return -1; - } + goto out_file; ini->buffer = (char*) malloc(fileSize + 2); if (!ini->buffer) - { - fclose(ini->fp); - ini->fp = NULL; - return -1; - } + goto out_file; if (fread(ini->buffer, fileSize, 1, ini->fp) != 1) - { - free(ini->buffer); - fclose(ini->fp); - ini->buffer = NULL; - ini->fp = NULL; - return -1; - } + goto out_buffer; fclose(ini->fp); ini->fp = NULL; @@ -119,6 +109,14 @@ int IniFile_Load_File(wIniFile* ini, const char* filename) ini->nextLine = strtok(ini->buffer, "\n"); return 1; + +out_buffer: + free(ini->buffer); + ini->buffer = NULL; +out_file: + fclose(ini->fp); + ini->fp = NULL; + return -1; } void IniFile_Load_Finish(wIniFile* ini) @@ -681,6 +679,7 @@ int IniFile_WriteFile(wIniFile* ini, const char* filename) { int length; char* buffer; + int ret = 1; buffer = IniFile_WriteBuffer(ini); @@ -700,13 +699,14 @@ int IniFile_WriteFile(wIniFile* ini, const char* filename) return -1; } - fwrite((void*) buffer, length, 1, ini->fp); + if (fwrite((void*) buffer, length, 1, ini->fp) != 1) + ret = -1; fclose(ini->fp); free(buffer); - return 1; + return ret; } wIniFile* IniFile_New() diff --git a/winpr/libwinpr/utils/lodepng/lodepng.c b/winpr/libwinpr/utils/lodepng/lodepng.c index ed4bb1482..4af9f6d8c 100644 --- a/winpr/libwinpr/utils/lodepng/lodepng.c +++ b/winpr/libwinpr/utils/lodepng/lodepng.c @@ -362,6 +362,7 @@ unsigned lodepng_load_file(unsigned char** out, size_t* outsize, const char* fil if(size && (*out)) (*outsize) = fread(*out, 1, (size_t)size, file); fclose(file); + if (*outsize != size) return 91; if(!(*out) && size) return 83; /*the above malloc failed*/ return 0; } @@ -370,11 +371,13 @@ unsigned lodepng_load_file(unsigned char** out, size_t* outsize, const char* fil unsigned lodepng_save_file(const unsigned char* buffer, size_t buffersize, const char* filename) { FILE* file; + int ret = 0; file = fopen(filename, "wb" ); if(!file) return 79; - fwrite((char*)buffer , 1 , buffersize, file); + if (fwrite((char*)buffer , 1 , buffersize, file) != buffersize) + ret = 91; fclose(file); - return 0; + return ret; } #endif /*LODEPNG_COMPILE_DISK*/ @@ -5894,6 +5897,7 @@ const char* lodepng_error_text(unsigned code) case 89: return "text chunk keyword too short or long: must have size 1-79"; /*the windowsize in the LodePNGCompressSettings. Requiring POT(==> & instead of %) makes encoding 12% faster.*/ case 90: return "windowsize must be a power of two"; + case 91: return "fwrite failed"; } return "unknown error code"; } diff --git a/winpr/libwinpr/utils/wlog/BinaryAppender.c b/winpr/libwinpr/utils/wlog/BinaryAppender.c index 3235868c9..50d9c86cd 100644 --- a/winpr/libwinpr/utils/wlog/BinaryAppender.c +++ b/winpr/libwinpr/utils/wlog/BinaryAppender.c @@ -131,6 +131,7 @@ int WLog_BinaryAppender_WriteMessage(wLog* log, wLogBinaryAppender* appender, wL int FileNameLength; int FunctionNameLength; int TextStringLength; + int ret = 1; if (!log || !appender || !message) return -1; @@ -171,11 +172,12 @@ int WLog_BinaryAppender_WriteMessage(wLog* log, wLogBinaryAppender* appender, wL Stream_SealLength(s); - fwrite(Stream_Buffer(s), MessageLength, 1, fp); + if (fwrite(Stream_Buffer(s), MessageLength, 1, fp) != 1) + ret = -1; Stream_Free(s, TRUE); - return 1; + return ret; } int WLog_BinaryAppender_WriteDataMessage(wLog* log, wLogBinaryAppender* appender, wLogMessage* message) diff --git a/winpr/libwinpr/utils/wlog/DataMessage.c b/winpr/libwinpr/utils/wlog/DataMessage.c index 892050425..407674378 100644 --- a/winpr/libwinpr/utils/wlog/DataMessage.c +++ b/winpr/libwinpr/utils/wlog/DataMessage.c @@ -32,6 +32,7 @@ int WLog_DataMessage_Write(char* filename, void* data, int length) { FILE* fp; fp = fopen(filename, "w+b"); + int ret = 0; if (!fp) { @@ -39,7 +40,8 @@ int WLog_DataMessage_Write(char* filename, void* data, int length) return -1; } - fwrite(data, length, 1, fp); + if (fwrite(data, length, 1, fp) != 1) + ret = -1; fclose(fp); - return 0; + return ret; } diff --git a/winpr/libwinpr/utils/wlog/PacketMessage.c b/winpr/libwinpr/utils/wlog/PacketMessage.c index e606dcaa1..eb1edfb7f 100644 --- a/winpr/libwinpr/utils/wlog/PacketMessage.c +++ b/winpr/libwinpr/utils/wlog/PacketMessage.c @@ -50,41 +50,28 @@ static int gettimeofday(struct timeval* tp, void* tz) } #endif -void Pcap_Read_Header(wPcap* pcap, wPcapHeader* header) +static BOOL Pcap_Read_Header(wPcap* pcap, wPcapHeader* header) { - if (pcap && pcap->fp) - fread((void*) header, sizeof(wPcapHeader), 1, pcap->fp); + if (pcap && pcap->fp && fread((void*) header, sizeof(wPcapHeader), 1, pcap->fp) == 1) + return TRUE; + return FALSE; } -void Pcap_Write_Header(wPcap* pcap, wPcapHeader* header) +/* currently unused code */ +# if 0 +static BOOL Pcap_Read_RecordHeader(wPcap* pcap, wPcapRecordHeader* record) { - if (pcap && pcap->fp) - fwrite((void*) header, sizeof(wPcapHeader), 1, pcap->fp); + if (pcap && pcap->fp && (fread((void*) record, sizeof(wPcapRecordHeader), 1, pcap->fp) == 1)) + return TRUE; + return FALSE; } -void Pcap_Read_RecordHeader(wPcap* pcap, wPcapRecordHeader* record) -{ - if (pcap && pcap->fp) - fread((void*) record, sizeof(wPcapRecordHeader), 1, pcap->fp); -} - -void Pcap_Write_RecordHeader(wPcap* pcap, wPcapRecordHeader* record) -{ - if (pcap && pcap->fp) - fwrite((void*) record, sizeof(wPcapRecordHeader), 1, pcap->fp); -} - -void Pcap_Write_RecordContent(wPcap* pcap, wPcapRecord* record) -{ - if (pcap && pcap->fp) - fwrite(record->data, record->length, 1, pcap->fp); -} - -BOOL Pcap_Read_Record(wPcap* pcap, wPcapRecord* record) +static BOOL Pcap_Read_Record(wPcap* pcap, wPcapRecord* record) { if (pcap && pcap->fp) { - Pcap_Read_RecordHeader(pcap, &record->header); + if (!Pcap_Read_RecordHeader(pcap, &record->header)) + return FALSE; record->length = record->header.incl_len; record->data = malloc(record->length); if (!record->data) @@ -100,13 +87,7 @@ BOOL Pcap_Read_Record(wPcap* pcap, wPcapRecord* record) return TRUE; } -void Pcap_Write_Record(wPcap* pcap, wPcapRecord* record) -{ - Pcap_Write_RecordHeader(pcap, &record->header); - Pcap_Write_RecordContent(pcap, record); -} - -void Pcap_Add_Record(wPcap* pcap, void* data, UINT32 length) +static BOOL Pcap_Add_Record(wPcap* pcap, void* data, UINT32 length) { wPcapRecord* record; struct timeval tp; @@ -115,7 +96,7 @@ void Pcap_Add_Record(wPcap* pcap, void* data, UINT32 length) { pcap->tail = (wPcapRecord*) calloc(1, sizeof(wPcapRecord)); if (!pcap->tail) - return; + return FALSE; pcap->head = pcap->tail; pcap->record = pcap->head; record = pcap->tail; @@ -124,7 +105,7 @@ void Pcap_Add_Record(wPcap* pcap, void* data, UINT32 length) { record = (wPcapRecord*) calloc(1, sizeof(wPcapRecord)); if (!record) - return; + return FALSE; pcap->tail->next = record; pcap->tail = record; } @@ -139,9 +120,10 @@ void Pcap_Add_Record(wPcap* pcap, void* data, UINT32 length) gettimeofday(&tp, 0); record->header.ts_sec = tp.tv_sec; record->header.ts_usec = tp.tv_usec; + return TRUE; } -BOOL Pcap_HasNext_Record(wPcap* pcap) +static BOOL Pcap_HasNext_Record(wPcap* pcap) { if (pcap->file_size - (ftell(pcap->fp)) <= 16) return FALSE; @@ -149,34 +131,59 @@ BOOL Pcap_HasNext_Record(wPcap* pcap) return TRUE; } -BOOL Pcap_GetNext_RecordHeader(wPcap* pcap, wPcapRecord* record) +static BOOL Pcap_GetNext_RecordHeader(wPcap* pcap, wPcapRecord* record) { - if (Pcap_HasNext_Record(pcap) != TRUE) + if (!Pcap_HasNext_Record(pcap) || !Pcap_Read_RecordHeader(pcap, &record->header)) return FALSE; - Pcap_Read_RecordHeader(pcap, &record->header); record->length = record->header.incl_len; return TRUE; } -BOOL Pcap_GetNext_RecordContent(wPcap* pcap, wPcapRecord* record) +static BOOL Pcap_GetNext_RecordContent(wPcap* pcap, wPcapRecord* record) { - if (pcap && pcap->fp) - { - fread(record->data, record->length, 1, pcap->fp); + if (pcap && pcap->fp && fread(record->data, record->length, 1, pcap->fp) == 1) return TRUE; - } return FALSE; } -BOOL Pcap_GetNext_Record(wPcap* pcap, wPcapRecord* record) +static BOOL Pcap_GetNext_Record(wPcap* pcap, wPcapRecord* record) { - if (Pcap_HasNext_Record(pcap) != TRUE) + if (!Pcap_HasNext_Record(pcap)) return FALSE; return Pcap_Read_Record(pcap, record); } +#endif + + +static BOOL Pcap_Write_Header(wPcap* pcap, wPcapHeader* header) +{ + if (pcap && pcap->fp && fwrite((void*) header, sizeof(wPcapHeader), 1, pcap->fp) == 1) + return TRUE; + return FALSE; +} + +static BOOL Pcap_Write_RecordHeader(wPcap* pcap, wPcapRecordHeader* record) +{ + if (pcap && pcap->fp && fwrite((void *) record, sizeof(wPcapRecordHeader), 1, pcap->fp) == 1) + return TRUE; + return FALSE; +} + +static BOOL Pcap_Write_RecordContent(wPcap* pcap, wPcapRecord* record) +{ + if (pcap && pcap->fp && fwrite(record->data, record->length, 1, pcap->fp) == 1) + return TRUE; + return FALSE; +} + +static BOOL Pcap_Write_Record(wPcap* pcap, wPcapRecord* record) +{ + return Pcap_Write_RecordHeader(pcap, &record->header) && + Pcap_Write_RecordContent(pcap, record); +} wPcap* Pcap_Open(char* name, BOOL write) { @@ -191,33 +198,45 @@ wPcap* Pcap_Open(char* name, BOOL write) pcap = (wPcap*) calloc(1, sizeof(wPcap)); - if (pcap) - { - pcap->name = name; - pcap->write = write; - pcap->record_count = 0; - pcap->fp = pcap_fp; + if (!pcap) + return NULL; - if (write) - { - pcap->header.magic_number = PCAP_MAGIC_NUMBER; - pcap->header.version_major = 2; - pcap->header.version_minor = 4; - pcap->header.thiszone = 0; - pcap->header.sigfigs = 0; - pcap->header.snaplen = 0xFFFFFFFF; - pcap->header.network = 1; /* ethernet */ - Pcap_Write_Header(pcap, &pcap->header); - } - else - { - fseek(pcap->fp, 0, SEEK_END); - pcap->file_size = (int) ftell(pcap->fp); - fseek(pcap->fp, 0, SEEK_SET); - Pcap_Read_Header(pcap, &pcap->header); - } + pcap->name = name; + pcap->write = write; + pcap->record_count = 0; + pcap->fp = pcap_fp; + + if (write) + { + pcap->header.magic_number = PCAP_MAGIC_NUMBER; + pcap->header.version_major = 2; + pcap->header.version_minor = 4; + pcap->header.thiszone = 0; + pcap->header.sigfigs = 0; + pcap->header.snaplen = 0xFFFFFFFF; + pcap->header.network = 1; /* ethernet */ + if (!Pcap_Write_Header(pcap, &pcap->header)) + goto out_fail; } + else + { + if (fseek(pcap->fp, 0, SEEK_END) < 0) + goto out_fail; + pcap->file_size = (int) ftell(pcap->fp); + if (pcap->file_size < 0) + goto out_fail; + if (fseek(pcap->fp, 0, SEEK_SET) < 0) + goto out_fail; + if (!Pcap_Read_Header(pcap, &pcap->header)) + goto out_fail; + } + return pcap; + +out_fail: + fclose(pcap_fp); + free(pcap); + return NULL; } void Pcap_Flush(wPcap* pcap) @@ -227,11 +246,13 @@ void Pcap_Flush(wPcap* pcap) while (pcap->record) { - Pcap_Write_Record(pcap, pcap->record); + if (!Pcap_Write_Record(pcap, pcap->record)) + return; pcap->record = pcap->record->next; } fflush(pcap->fp); + return; } void Pcap_Close(wPcap* pcap) @@ -244,26 +265,28 @@ void Pcap_Close(wPcap* pcap) free(pcap); } -int WLog_PacketMessage_Write_EthernetHeader(wPcap* pcap, wEthernetHeader* ethernet) +static BOOL WLog_PacketMessage_Write_EthernetHeader(wPcap* pcap, wEthernetHeader* ethernet) { wStream* s; BYTE buffer[14]; + BOOL ret = TRUE; if (!pcap || !pcap->fp || !ethernet) - return -1; + return FALSE; s = Stream_New(buffer, 14); if (!s) - return -1; + return FALSE; Stream_Write(s, ethernet->Destination, 6); Stream_Write(s, ethernet->Source, 6); Stream_Write_UINT16_BE(s, ethernet->Type); - fwrite(buffer, 14, 1, pcap->fp); + if (fwrite(buffer, 14, 1, pcap->fp) != 1) + ret = FALSE; Stream_Free(s, FALSE); - return 0; + return ret; } -UINT16 IPv4Checksum(BYTE* ipv4, int length) +static UINT16 IPv4Checksum(BYTE* ipv4, int length) { UINT16 tmp16; long checksum = 0; @@ -285,17 +308,18 @@ UINT16 IPv4Checksum(BYTE* ipv4, int length) return (UINT16)(~checksum); } -int WLog_PacketMessage_Write_IPv4Header(wPcap* pcap, wIPv4Header* ipv4) +static BOOL WLog_PacketMessage_Write_IPv4Header(wPcap* pcap, wIPv4Header* ipv4) { wStream* s; BYTE buffer[20]; + int ret = TRUE; if (!pcap || !pcap->fp || !ipv4) - return -1; + return FALSE; s = Stream_New(buffer, 20); if (!s) - return -1; + return FALSE; Stream_Write_UINT8(s, (ipv4->Version << 4) | ipv4->InternetHeaderLength); Stream_Write_UINT8(s, ipv4->TypeOfService); Stream_Write_UINT16_BE(s, ipv4->TotalLength); @@ -310,22 +334,24 @@ int WLog_PacketMessage_Write_IPv4Header(wPcap* pcap, wIPv4Header* ipv4) Stream_Rewind(s, 10); Stream_Write_UINT16(s, ipv4->HeaderChecksum); Stream_Seek(s, 8); - fwrite(buffer, 20, 1, pcap->fp); + if (fwrite(buffer, 20, 1, pcap->fp) != 1) + ret = FALSE; Stream_Free(s, FALSE); - return 0; + return ret; } -int WLog_PacketMessage_Write_TcpHeader(wPcap* pcap, wTcpHeader* tcp) +static BOOL WLog_PacketMessage_Write_TcpHeader(wPcap* pcap, wTcpHeader* tcp) { wStream* s; BYTE buffer[20]; + BOOL ret = TRUE; if (!pcap || !pcap->fp || !tcp) - return -1; + return FALSE; s = Stream_New(buffer, 20); if (!s) - return -1; + return FALSE; Stream_Write_UINT16_BE(s, tcp->SourcePort); Stream_Write_UINT16_BE(s, tcp->DestinationPort); Stream_Write_UINT32_BE(s, tcp->SequenceNumber); @@ -337,10 +363,13 @@ int WLog_PacketMessage_Write_TcpHeader(wPcap* pcap, wTcpHeader* tcp) Stream_Write_UINT16_BE(s, tcp->UrgentPointer); if (pcap->fp) - fwrite(buffer, 20, 1, pcap->fp); + { + if (fwrite(buffer, 20, 1, pcap->fp) != 1) + ret = FALSE; + } Stream_Free(s, FALSE); - return 0; + return ret; } static UINT32 g_InboundSequenceNumber = 0; @@ -445,11 +474,12 @@ int WLog_PacketMessage_Write(wPcap* pcap, void* data, DWORD length, DWORD flags) gettimeofday(&tp, 0); record.header.ts_sec = tp.tv_sec; record.header.ts_usec = tp.tv_usec; - Pcap_Write_RecordHeader(pcap, &record.header); - WLog_PacketMessage_Write_EthernetHeader(pcap, ðernet); - WLog_PacketMessage_Write_IPv4Header(pcap, &ipv4); - WLog_PacketMessage_Write_TcpHeader(pcap, &tcp); - Pcap_Write_RecordContent(pcap, &record); + if (!Pcap_Write_RecordHeader(pcap, &record.header) || + !WLog_PacketMessage_Write_EthernetHeader(pcap, ðernet) || + !WLog_PacketMessage_Write_IPv4Header(pcap, &ipv4) || + !WLog_PacketMessage_Write_TcpHeader(pcap, &tcp) || + !Pcap_Write_RecordContent(pcap, &record)) + return -1; fflush(pcap->fp); return 0; } diff --git a/winpr/tools/makecert/makecert.c b/winpr/tools/makecert/makecert.c index 82fc42065..fbec207f6 100644 --- a/winpr/tools/makecert/makecert.c +++ b/winpr/tools/makecert/makecert.c @@ -584,7 +584,8 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa goto out_fail; length = offset; - fwrite((void*) x509_str, length, 1, fp); + if (fwrite((void*) x509_str, length, 1, fp) != 1) + goto out_fail; } else @@ -639,7 +640,8 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa length = offset; - fwrite((void*) x509_str, length, 1, fp); + if (fwrite((void*) x509_str, length, 1, fp) != 1) + goto out_fail; free(x509_str); x509_str = NULL; @@ -696,7 +698,8 @@ int makecert_context_output_certificate_file(MAKECERT_CONTEXT* context, char* pa length = offset; - fwrite((void*) x509_str, length, 1, fp); + if (fwrite((void*) x509_str, length, 1, fp) != 1) + goto out_fail; } } @@ -717,12 +720,15 @@ out_fail: int makecert_context_output_private_key_file(MAKECERT_CONTEXT* context, char* path) { - FILE* fp; + FILE* fp = NULL; int status; int length; int offset; - char* filename; - char* fullpath; + char* filename = NULL; + char* fullpath = NULL; + int ret = -1; + BIO* bio = NULL; + BYTE* x509_str = NULL; if (!context->crtFormat) return 1; @@ -740,7 +746,6 @@ int makecert_context_output_private_key_file(MAKECERT_CONTEXT* context, char* pa return -1; strcpy(filename, context->output_file); strcpy(&filename[length], ".key"); - length = strlen(filename); if (path) fullpath = GetCombinedPath(path, filename); @@ -748,99 +753,77 @@ int makecert_context_output_private_key_file(MAKECERT_CONTEXT* context, char* pa fullpath = _strdup(filename); if (!fullpath) - { - free(filename); - return -1; - } + goto out_fail; fp = fopen(fullpath, "w+"); + if (!fp) + goto out_fail; - if (fp) + bio = BIO_new(BIO_s_mem()); + + if (!bio) + goto out_fail; + + if (!PEM_write_bio_PrivateKey(bio, context->pkey, NULL, NULL, 0, NULL, NULL)) + goto out_fail; + + offset = 0; + length = 2048; + x509_str = (BYTE*) malloc(length); + if (!x509_str) + goto out_fail; + + status = BIO_read(bio, x509_str, length); + + if (status < 0) + goto out_fail; + + offset += status; + + while (offset >= length) { - BIO* bio; - BYTE* x509_str; + int new_len; + BYTE *new_str; - bio = BIO_new(BIO_s_mem()); - - if (!bio) + new_len = length * 2; + new_str = (BYTE*) realloc(x509_str, new_len); + if (!new_str) { - free (filename); - free(fullpath); - fclose(fp); - return -1; + status = -1; + break; } - status = PEM_write_bio_PrivateKey(bio, context->pkey, NULL, NULL, 0, NULL, NULL); + length = new_len; + x509_str = new_str; - offset = 0; - length = 2048; - x509_str = (BYTE*) malloc(length); - if (!x509_str) - { - free (filename); - free(fullpath); - fclose(fp); - return -1; - } + status = BIO_read(bio, &x509_str[offset], length); - status = BIO_read(bio, x509_str, length); - if (status < 0) - { - free (filename); - free(fullpath); - fclose(fp); - return -1; - } - + break; + offset += status; - - while (offset >= length) - { - int new_len; - BYTE *new_str; - - new_len = length * 2; - new_str = (BYTE*) realloc(x509_str, new_len); - if (!new_str) - { - status = -1; - break; - } - - length = new_len; - x509_str = new_str; - - status = BIO_read(bio, &x509_str[offset], length); - - if (status < 0) - break; - - offset += status; - } - - if (status < 0) - { - free (filename); - free(fullpath); - fclose(fp); - return -1; - } - - length = offset; - - fwrite((void*) x509_str, length, 1, fp); - - free(x509_str); - BIO_free(bio); - - fclose(fp); } + if (status < 0) + goto out_fail; + + length = offset; + + if (fwrite((void*) x509_str, length, 1, fp) != 1) + goto out_fail; + + ret = 1; + +out_fail: + if (fp) + fclose(fp); + if (bio) + BIO_free(bio); + free(x509_str); free(filename); free(fullpath); - return 1; + return ret; } int makecert_context_process(MAKECERT_CONTEXT* context, int argc, char** argv)