Gents,
The intention was to always try and produce an image with a very small
foot-print. Some checks etc were not added because of this. The current
working version has changed as a result of adding some additional features and
some of these comments have already been modified.
The issues with accessing the micron via the UART is a difficult one. What do
you do if it does not respond to the write? The reads are handled as they are
important. UART writes, as with the nature of the device, are FIFO and
generally fire and forget. Some systems this is not an issue as the same
commands are generally fired again and can be picked up next time. These are
not. Some commands, user types, are issued the once and are expected to
succeed. I will make some changes to the UART write logic to support this.
I hope this answers this outstanding bug item.
Regards,
Bob
--- On Thu, 1/29/09, Per Andersson <avtob...@gmail.com> wrote:
From: Per Andersson <avtob...@gmail.com>
Subject: Re: Bug#513353: Misc programming oddities might have security
implications
To: "Loïc Minier" <l...@dooz.org>, 513...@bugs.debian.org
Cc: lbw...@yahoo.com
Date: Thursday, January 29, 2009, 8:35 PM
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
>
>
>
