Bob, what do you think about these remarks?
Best regards, Per On Wed, Jan 28, 2009 at 11:06 AM, Loïc Minier <l...@dooz.org> wrote: > Package: micro-evtd > Version: 3.3.3-6+lenny3 > Severity: important > Tags: security > > Hey, > > I was reading the micro-evtd source, and found some slightly scary > issues; first with these warnings: > micro_evtd.c: In function 'reset': > micro_evtd.c:240: warning: ignoring return value of 'write', declared with > attribute warn_unused_result > micro_evtd.c:244: warning: ignoring return value of 'read', declared with > attribute warn_unused_result > micro_evtd.c: In function 'writeUART': > micro_evtd.c:310: warning: ignoring return value of 'write', declared with > attribute warn_unused_result > micro_evtd.c:316: warning: ignoring return value of 'write', declared with > attribute warn_unused_result > > The read()/write() error checking is probably not a big issue in real > life, but it would probably be best to abort subsequent reads/writes > when one of them fails (except in reset() perhaps). > > micro_evtd.c: In function 'execute_command2': > micro_evtd.c:416: warning: ignoring return value of 'system', declared with > attribute warn_unused_result > > Not a big deal, but might be worth logging? > > micro_evtd.c: In function 'parse_configuration': > micro_evtd.c:1028: warning: format not a string literal and no format > arguments > > That's really trivial to fix by changing: > syslog(LOG_INFO, message); > into: > syslog(LOG_INFO, "%s", message); > but this remains scary: :-/ > sprintf(message, "%s-%02d/%02d %02d:%02d", message, ...); > > > Finally, the /tmp usage to run arbitrary commands scares me the most: > - AFAICT, mkdir /tmp/micro_evtd is unsecure > - /usr/sbin/micro_evtd.event is then copied into it unconditionally > (even if the dir aleady existed) > (So I could create /tmp/micro_evtd and a > /tmp/micro_evtd/micro_evtd.event -> /etc/passwd symlink and clobber any > file on startup?) > - strTmpPath seems to be able to overflow its buffer; the upstream > declaration is: > char strTmpPath[20]="/tmp"; > which is then used as follows: > sprintf( strTmpPath, "%s", pos); > with pos coming from a bunch of string parsin routines, and being set > in numerous places with sscanf() calls... > - there's a Debian patch to set strTmpPath to: > char strTmpPath[20]="/tmp/micro_evtd"; > I'm not sure this is long enough anymore. > > NB: strTmpPath() is used in execute_command2() whenver not running the > CP_SCRIPT. > > HTH, > -- > Loïc Minier > > > -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
