[opensuse-packaging] fcron Bufferoverflow
Hello Mates, ATM i'm fixing the last issues from fcron. Now i'm getting: I: A function overflows or underflows an array access. This could be a real error, but occasionaly this condition is also misdetected due to loop unrolling or strange pointer handling. So this is warning only, please review. W: fcron arraysubscript fileconf.c:1403, 1407 I: Statement might be overflowing a buffer in strncat. Common mistake: BAD: strncat(buffer,charptr,sizeof(buffer)) is wrong, it takes the left over size as 3rd argument GOOD: strncat(buffer,charptr,sizeof(buffer)-strlen(buffer)-1) E: fcron bufferoverflowstrncat socket.c:297 fileconf.c: if ( ! is_freq_hrs(cl->cl_option) ) { bit_ffc(cl->cl_hrs, s_hrs, &j); <-- 1403 if ( j != -1 && j < s_hrs) goto ok; } if ( ! is_freq_days(cl->cl_option) ) { bit_ffc(cl->cl_days, s_days, &j); <---1407 if ( j != -1 && j < s_days) { if ( is_dayand(cl->cl_option) ) goto ok; socket.c: #define Test_add_field(FIELD_NB, FIELD_STR) \ if ( (bit_test(details, FIELD_NB)) ) { \ strncat(fields, FIELD_STR, sizeof(fields)-1 - len); \ len += (sizeof(FIELD_STR)-1); \ } #define Add_field(FIELD_STR) \ strncat(fields, FIELD_STR, sizeof(fields) - len); \ len += (sizeof(FIELD_STR)-1); void print_fields(int fd, unsigned char *details) /* print a line describing the field types used in print_line() */ { char fields[TERM_LEN]; char field_user[] = " USER "; char field_id[] = "ID "; char field_rq[] = " R&Q "; char field_options[] = " OPTIONS "; char field_schedule[] = " SCHEDULE "; char field_until[] = " LAVG 1,5,15 UNTIL STRICT"; char field_pid[] = " PID "; char field_index[] = " INDEX"; char field_cmd[] = " CMD"; char field_endline[] = "\n"; int len = 0; fields[0] = '\0'; Add_field(field_id); <-- Line 297 Test_add_field(FIELD_USER, field_user); Test_add_field(FIELD_PID, field_pid); Test_add_field(FIELD_INDEX, field_index); Test_add_field(FIELD_RQ, field_rq); Test_add_field(FIELD_OPTIONS, field_options); Test_add_field(FIELD_LAVG, field_until); Test_add_field(FIELD_SCHEDULE, field_schedule); Add_field(field_cmd); Add_field(field_endline); fields[TERM_LEN-1] = '\0'; Anyone know, how to fix it? -- Sincerely yours Sascha Manns open-slx GmbH openSUSE Member openSUSE Community & Support Agent openSUSE Ambassador Web: http://www.open-slx.de openSUSE Marketing Team Web: http://www.open-slx.com openSUSE Build Service openSUSE Medical Team Web: http://saschamanns.gulli.to Blog: http://saigkill.wordpress.com ClaimID: http://claimid.com/saigkill
On Wed, 2 Dec 2009 21:24:36 +0100, you wrote:
ATM i'm fixing the last issues from fcron.
I'll have a look tomorrow. I guess it's home:saigkill/fcron? Philipp -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
Am Donnerstag 03 Dezember 2009 01:48:57 wrote Philipp Thomas:
On Wed, 2 Dec 2009 21:24:36 +0100, you wrote:
ATM i'm fixing the last issues from fcron.
I'll have a look tomorrow. I guess it's home:saigkill/fcron? Yes, that's it.
-- Sincerely yours Sascha Manns open-slx GmbH openSUSE Member openSUSE Community & Support Agent openSUSE Ambassador Web: http://www.open-slx.de openSUSE Marketing Team Web: http://www.open-slx.com openSUSE Build Service openSUSE Medical Team Web: http://saschamanns.gulli.to Blog: http://saigkill.wordpress.com ClaimID: http://claimid.com/saigkill -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
On 02/12/09 17:24, Sascha 'saigkill' Manns wrote:
I: Statement might be overflowing a buffer in strncat. Common mistake: BAD: strncat(buffer,charptr,sizeof(buffer)) is wrong, it takes the left over size as 3rd argument GOOD: strncat(buffer,charptr,sizeof(buffer)-strlen(buffer)-1) E: fcron bufferoverflowstrncat socket.c:297
see https://buildsecurityin.us-cert.gov/daisy/bsi-rules/home/g1/853-BSI.html. -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
Sascha 'saigkill' Manns wrote:
I: Statement might be overflowing a buffer in strncat. Common mistake: BAD: strncat(buffer,charptr,sizeof(buffer)) is wrong, it takes the left over size as 3rd argument GOOD: strncat(buffer,charptr,sizeof(buffer)-strlen(buffer)-1) E: fcron bufferoverflowstrncat socket.c:297 [...] strncat(fields, FIELD_STR, sizeof(fields) - len); \
strncat has a braindead api. In the worst case it adds one byte more than the specified length. So for example if FIELD_STR == sizeof(fields) and len == 0 it would overflow the buffer by one zero byte. Add a -1 to the calculation. cu Ludwig -- (o_ Ludwig Nussel //\ V_/_ http://www.suse.de/ SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nuernberg) -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
On 03/12/09 05:44, Ludwig Nussel wrote:
strncat has a braindead api. In the worst case it adds one byte more than the specified length. So for example if FIELD_STR == sizeof(fields) and len == 0 it would overflow the buffer by one zero byte. Add a -1 to the calculation.
if the application uses "glib" library, you may also try g_strconcat (). -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
* Ludwig Nussel (ludwig.nussel@suse.de) [20091203 09:44]:
than the specified length. So for example if FIELD_STR == sizeof(fields) and len == 0 it would overflow the buffer by one zero byte. Add a -1 to the calculation.
This is the code: define Test_add_field(FIELD_NB, FIELD_STR) \ if ( (bit_test(details, FIELD_NB)) ) { \ strncat(fields, FIELD_STR, sizeof(fields)-1 - len); \ len += (sizeof(FIELD_STR)-1); \ } #define Add_field(FIELD_STR) \ strncat(fields, FIELD_STR, sizeof(fields) - len); \ len += (sizeof(FIELD_STR)-1); void print_fields(int fd, unsigned char *details) /* print a line describing the field types used in print_line() */ { char fields[TERM_LEN]; char field_user[] = " USER "; char field_id[] = "ID "; char field_rq[] = " R&Q "; char field_options[] = " OPTIONS "; char field_schedule[] = " SCHEDULE "; char field_until[] = " LAVG 1,5,15 UNTIL STRICT"; char field_pid[] = " PID "; char field_index[] = " INDEX"; char field_cmd[] = " CMD"; char field_endline[] = "\n"; int len = 0; fields[0] = '\0'; Add_field(field_id); /* <--- This is the line in question */ Test_add_field(FIELD_USER, field_user); Shouldn't Add _field be defined as define Add_field(FIELD_STR) \ strncat(fields, FIELD_STR, sizeof(fields)-1 - len); \ len += (sizeof(FIELD_STR)-1); ? Philipp -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-packaging+help@opensuse.org
participants (5)
-
Cristian Rodríguez
-
Ludwig Nussel
-
Philipp Thomas
-
Philipp Thomas
-
Sascha 'saigkill' Manns