From: Kevin Day <kevin@kevux.org>
Date: Sun, 11 Mar 2012 02:58:10 +0000 (-0600)
Subject: Bugfix: fix logic flaws with file input buffers
X-Git-Tag: 0.3.0~96
X-Git-Url: https://git.kevux.org/?a=commitdiff_plain;h=27abc16984b76ddaad6bdec352087fd6e8f6c7da;p=fll

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 2e58db2..12257b5 100644
--- 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 1e69f74..347d38a 100644
--- 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)){