[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    compiling with -Wformat-security
From:       Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn () axis ! com>
Date:       2008-07-26 14:49:10
Message-ID: 0807131526100.23610 () somehost
[Download RAW message or body]

Got me going while googling for "format string vulnerability" in an
attempt to fresh up some distant memories :)

I added the -Wformat-security compiler option to Makefile.flags and fired
up a rebuild.  Showed up most of the generated warnings look similar to
this:

coreutils/df.c:80: warning: format not a string literal and no format arguments

have to do with calling the following functions:

  bb_error_msg
  bb_error_msg_and_die
  bb_perror_msg
  bb_perror_msg_and_die

without a format specifier.  Typical case:

  bb_perror_msg_and_die(bb_msg_read_error);

Adding a format specifier (there must be a smarter way) to all those
instances, like:

  bb_perror_msg_and_die("%s", bb_msg_read_error);

kill the warnings, but also adds some 5 bytes per call:

function                                             old     new   delta
singlemount                                         4339    4350     +11
create_icmp_socket                                    65      75     +10
create_icmp6_socket                                   65      75     +10
tr_main                                              472     481      +9
mount_it_now                                         166     175      +9
do_sethostname                                       132     141      +9
uuencode_main                                        334     342      +8
chpasswd_main                                        307     315      +8
arpping                                              394     402      +8
tail_read                                            133     139      +6
sysctl_display_all                                   142     148      +6
malloc_or_warn                                        37      43      +6
bb_full_fd_action                                    233     239      +6
zcip_main                                           1415    1420      +5
xstrdup                                               28      33      +5
xsetenv                                               27      32      +5
xrealloc                                              32      37      +5
xputchar                                              25      30      +5
xmalloc                                               30      35      +5
xfflush_stdout                                        27      32      +5
xasprintf                                             44      49      +5
uuencode                                             242     247      +5
puts_maybe_newline                                   144     149      +5
od_main                                             2228    2233      +5
fflush_stdout_and_exit                                55      60      +5
expand_main                                          672     677      +5
error_exit                                            20      25      +5
df_main                                              580     585      +5
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 28/0 up/down: 181/0)            Total: 181 bytes
   text    data     bss     dec     hex filename
 500848    2078   22028  524954   8029a busybox_old
 501029    2078   22028  525135   8034f busybox_unstripped

Most (if not all) of those calls to bb_p?error_msg* are safe, as the
arguments are _not_ user entered strings, but internal char vectors.
Other calls generating warnings I found:

  libbb/dump.c:596:       printf(pr->fmt);
  libbb/dump.c:648:       (void) printf(pr->fmt);
  shell/ash.c:3731:       col = fmtstr(s, 32, strsignal(st));

Thoughts?


Cheers,

-- 
Cristian
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic