Bugfix: fix logic flaws with file input buffers

The first mistake made was that when adding the fread result to the buffer, the actual size of each read byte needs to be taken into mind.
If the characters were wide characters with a byte size of 2 instead of 1, then the buffer->used counter would increase at a rate of 2x of what was actually in use.

The second mistake made was that the fread max read size was being set to the entire buffer size instead of what was available.
This should have been producing some sort of buffer overflow, but none was reported.
Instead of request a read of (buffer->size - 1), only request a size
equal to that of the available allocated space (buffer->size - buffer->used - 1).

The third mistake made was not performing sanity checking on the buffer->used and buffer->size variables.

diff --git a/level_0/f_file/c/file.c b/level_0/f_file/c/file.c
index 2e58db26b5a8e6e2660148bd8f15853ae487f113..12257b5b33f96d3afdca9b7d7ac80cd0aa29d5e0 100644 (file)
--- a/level_0/f_file/c/file.c
+++ b/level_0/f_file/c/file.c
@@ -74,7 +74,9 @@ extern "C"{
 #ifndef _di_f_file_read_
   f_return_status f_file_read(f_file *file_information, f_dynamic_string *buffer, const f_file_position location){
     #ifndef _di_level_0_parameter_checking_
-      if (file_information        == f_null) return f_invalid_parameter;
+      if (file_information == f_null)       return f_invalid_parameter;
+      if (buffer->used     >= buffer->size) return f_invalid_parameter;
+
       if (location.buffer_start    < 0)      return f_invalid_parameter;
       if (location.file_start      < 0)      return f_invalid_parameter;
       if (location.total_elements  < 0)      return f_invalid_parameter;
@@ -103,7 +105,7 @@ extern "C"{
 
     // now do the actual read
     if (location.total_elements == 0){
-      result = fread(buffer->string + location.buffer_start, file_information->byte_size, buffer->size - 1, file_information->file);
+      result = fread(buffer->string + location.buffer_start, file_information->byte_size, buffer->size - buffer->used - 1, file_information->file);
     } else {
       result = fread(buffer->string + location.buffer_start, file_information->byte_size, location.total_elements, file_information->file);
     }
@@ -114,7 +116,7 @@ extern "C"{
     // now save how much of our allocated buffer is actually used
     // also make sure that we aren't making used space vanish
     if (location.buffer_start + result > buffer->used){
-      buffer->used = location.buffer_start + result;
+      buffer->used = location.buffer_start + (result / file_information->byte_size);
     }
 
     // append an EOS only when the total elements were set to 0
@@ -134,7 +136,8 @@ extern "C"{
 #ifndef _di_f_file_read_fifo_
   f_return_status f_file_read_fifo(f_file *file_information, f_dynamic_string *buffer){
     #ifndef _di_level_0_parameter_checking_
-      if (file_information == f_null) return f_invalid_parameter;
+      if (file_information == f_null)       return f_invalid_parameter;
+      if (buffer->used     >= buffer->size) return f_invalid_parameter;
     #endif // _di_level_0_parameter_checking_
 
     if (file_information->file == 0) return f_file_not_open;
@@ -142,12 +145,12 @@ extern "C"{
     f_s_int result = 0;
 
     // now do the actual read
-    result = fread(buffer->string + buffer->used, file_information->byte_size, buffer->size - 1, file_information->file);
+    result = fread(buffer->string + buffer->used, file_information->byte_size, buffer->size - buffer->used - 1, file_information->file);
 
     if (file_information->file == 0)         return f_file_read_error;
     if (ferror(file_information->file) != 0) return f_file_read_error;
 
-    buffer->used += result;
+    buffer->used += (result / file_information->byte_size);
 
     // make sure to communicate that we are done without a problem and the eof was reached
     if (feof(file_information->file)){

diff --git a/level_1/fl_file/c/file.c b/level_1/fl_file/c/file.c
index 1e69f747205fcdd9c01ab7085dcf4e597ccb00da..347d38af68f759a1bcd032b8cf606eb1ebde5a89 100644 (file)
--- a/level_1/fl_file/c/file.c
+++ b/level_1/fl_file/c/file.c
@@ -36,7 +36,7 @@ extern "C"{
 
     // populate the buffer
     do{
-      if (buffer->size < size){
+      if (buffer->size <= size){
         f_resize_dynamic_string(status, (*buffer), size);
 
         if (f_macro_test_for_allocation_errors(status)){
@@ -80,7 +80,7 @@ extern "C"{
 
     // populate the buffer
     do {
-      if (buffer->size < size){
+      if (buffer->size <= size){
         f_resize_dynamic_string(status, (*buffer), size);
 
         if (f_macro_test_for_allocation_errors(status)){