Hi,

http://sources.debian.net/src/trafficserver/3.0.5-1/mgmt/tools/SysAPI.cc
>     NOWARN_UNUSED_RETURN(system("/bin/mv -f /tmp/shadow /etc/shadow"));

Won't that reset the shadow file's ownership to root:root?  If default
umask is 027, the file won't be readable any more by the shadow group;
won't that break login if this code is ever used?

(Or if umask is less strict for some reason, the file becomes
world-readable I suppose?)


And there is plenty more /tmp abuse in that file - here it tries to
delete the file, but there is still a race before creating/writing to it:

>     ink_strncpy(command, "/sbin/route -n > /tmp/route_status", 
> sizeof(command));
>     remove(tmp_file);
>     if (system(command) == -1) {

And again:

>       const char *tmp_file = "/tmp/dhcp_status";

>       ink_strncpy(command, "/sbin/ifconfig ", sizeof(command));
>       strncat(command, interface, (sizeof(command) - strlen(command) - 1));
>       strncat(command, " >", (sizeof(command) - strlen(command) - 1));
>       strncat(command, tmp_file, (sizeof(command) - strlen(command) - 1));
> 
>       remove(tmp_file);
>       if (system(command) == -1) {

(Also wondering why there's an ink_strncpy but using regular strncat).

Another, this time without deleting the file if it already exists:

>   tmp = fopen("/tmp/zonetab.tmp", "w");

Followed by:

>   remove("/tmp/zonetab");
>   NOWARN_UNUSED_RETURN(system("/bin/sort /tmp/zonetab.tmp > /tmp/zonetab"));

...and after doing all of this, /tmp/zonetab isn't even used!?!


Also there is plenty of code here not likely to work at all on a Debian
system.  /etc/sysconfig is quite specific to Red Hat and derivatives:

>   return (!find_value("/etc/sysconfig/clock", "ZONE", timezone, timezone_len, 
> "=", 0));

>   ink_strncpy(ifcfg, "/etc/sysconfig/network-scripts/ifcfg-", sizeof(ifcfg));

Debian doesn't have chkconfig;  we'd probably want to use service and
update-rc.d here:

>   status = system("/etc/init.d/ntpd stop");
>   status = system("/sbin/chkconfig --level 2345 ntpd off");

Who wants to do a thing like that anyway?


>   net_device = fopen("/proc/net/dev", "r");

>       if (strstr(buffer, "eth"))  // only counts the eth interface

What if trafficserver is to run on an interface not called eth?  Some
virtual environments might call it veth or vether for example.  Maybe
you want to use it on a wlan interface, ppp, or perhaps on lo0?


One final curio for amusement.  A past Coverity scan must have already
pointed out the sillyness of this, but:

>   // coverity[fs_check_call]
>   if (access(pathname, R_OK)) {
>     return find;
>   }
>   // coverity[toctou]
>   if ((fp = fopen(pathname, "r")) != NULL) {
>     ...
>   }
>   return find;

Why bother with access()?  If a fopen() failure would do the exact same
thing anyway...


[ Hoping this whole file isn't needed, and can simply go away :) ]

Regards,
-- 
Steven Chamberlain
ste...@pyro.eu.org

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to