avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
734 stars 136 forks source link

[patch #9820] Fix some out-of-bounds/uninitialized issues #746

Closed avrs-admin closed 2 years ago

avrs-admin commented 2 years ago

Sun 23 Jun 2019 11:55:52 AM UTC

avrdude_stk500v2.c_potential_out_of_bounds.patch: Fix for loop missing div by sizeof first element + index var will be outside array range if no match found.

avrdude_pickit2.c_out_of_bounds.patch: Fix assigning PGM_DESCLEN (=80) bytes to pgm->type (32 bytes). Most likely a mixup between type and desc fields. Comment states intention to get description, not type.

avrdude_stk500v2.c_potentially_uninitialized.patch: Fix variables being uninitialized in call to avrdude_message if stk500v2_getparm fails.

avrdude_avr.c_potential_div_by_zero.patch: Fix missing check for page_size > 0 potentially resulting in div by zero in modulo page_size.

file #47130: avrdude_avr.c_potential_div_by_zero.patch file #47129: avrdude_pickit2.c_out_of_bounds.patch file #47128: avrdude_stk500v2.c_potentially_uninitialized.patch file #47127: avrdude_stk500v2.c_potential_out_of_bounds.patch

This issue was migrated from https://savannah.nongnu.org/patch/?9820

avrs-admin commented 2 years ago

Joerg Wunsch Thu 10 Sep 2020 09:24:56 PM UTC

Regarding the divide by 0 patch, I think the following patch is more versatile:

Index: avrpart.c
===================================================================
--- avrpart.c   (revision 1435)
+++ avrpart.c   (working copy)
@@ -254,6 +254,7 @@
}

memset(m, 0, sizeof(*m));
+  m->page_size = 1; // ensure not 0

return m;
}
Index: config_gram.y
===================================================================
--- config_gram.y       (revision 1435)
+++ config_gram.y       (working copy)
@@ -1310,7 +1310,11 @@

K_PAGE_SIZE       TKN_EQUAL TKN_NUMBER
{
-      current_mem->page_size = $3->value.number;
+      int ps = $3->value.number;
+      if (ps <= 0)
+        avrdude_message(MSG_NOTICE, "invalid page size %d, ignored", ps);
+      else
+        current_mem->page_size = ps;
free_token($3);
} |

This ensures the page size is always at least 1, so any modulo operation with it will work - regardless of where it happens.