Re: [PATCH 3/3] wall: Temporarily drop privileges when opening files
On Tuesday 08 October 2013 02:02:33 Ryan Mallon wrote: The wall applet is setuid and currently does no checking of the real user's read access to the message file. This allows the wall applet to be used to display files which are not readable by an unprivileged user. For example: $ wall /etc/shadow $ wall /proc/vmallocinfo Fix this by temporarily dropping privileges before opening the file. Signed-off-by: Ryan Mallon rmal...@gmail.com --- miscutils/wall.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/miscutils/wall.c b/miscutils/wall.c index 762f53b..0f9d046 100644 --- a/miscutils/wall.c +++ b/miscutils/wall.c @@ -22,7 +22,24 @@ int wall_main(int argc UNUSED_PARAM, char **argv) { struct utmp *ut; char *msg; - int fd = argv[1] ? xopen(argv[1], O_RDONLY) : STDIN_FILENO; + int fd = STDIN_FILENO; + + if (argv[1]) { + /* + * This applet is setuid. Temporarily drop privileges to the + * real user when opening the file. + */ + uid_t old_euid = geteuid(); + gid_t old_egid = getegid(); + + xsetegid(getgid()); + xseteuid(getuid()); + + fd = xopen(argv[1], O_RDONLY); + + xseteuid(old_euid); + xsetegid(old_egid); + } msg = xmalloc_read(fd, NULL); if (ENABLE_FEATURE_CLEAN_UP argv[1]) Hi, seems to me that now we can move all this stuff to libbb as there are already two applets that use it. int xopen_as_user(char *path) { int fd; uid_t old_euid = geteuid(); gid_t old_egid = getegid(); xsetegid(getgid()); xseteuid(getuid()); fd = xopen(path, O_RDONLY); xseteuid(old_euid); xsetegid(old_egid); return fd } Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 3/3] wall: Temporarily drop privileges when opening files
On Tuesday 08 October 2013 02:02, Ryan Mallon wrote: The wall applet is setuid and currently does no checking of the real user's read access to the message file. This allows the wall applet to be used to display files which are not readable by an unprivileged user. For example: $ wall /etc/shadow $ wall /proc/vmallocinfo Fix this by temporarily dropping privileges before opening the file. Applied all three patches (with small modifications). Thanks! ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 1/2] wget: make default timeout configurable, remove a duplicate line
Hi, Small housekeeping for wget. - Lauri -- http://www.fastmail.fm - One of many happy users: http://www.fastmail.fm/help/overview_quotes.html From 13755e22b0dc9655d5e5d05c2a0d06873a8ef526 Mon Sep 17 00:00:00 2001 From: Lauri Kasanen cur...@operamail.com Date: Tue, 8 Oct 2013 16:40:20 +0300 Subject: [PATCH 1/2] wget: make default timeout configurable, remove a duplicate line Signed-off-by: Lauri Kasanen cur...@operamail.com --- networking/Config.src | 8 networking/wget.c | 3 +-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/networking/Config.src b/networking/Config.src index e1ae0c9..0a483ae 100644 --- a/networking/Config.src +++ b/networking/Config.src @@ -981,6 +981,14 @@ config FEATURE_WGET_TIMEOUT connection initialization). When FEATURE_WGET_LONG_OPTIONS is also enabled, the --timeout option will work in addition to -T. +config WGET_DEFAULT_TIMEOUT + int Default wget timeout + default 900 + range 1 2000 + depends on FEATURE_WGET_TIMEOUT + help + The default time, in seconds, to wait before wget gives up. + config ZCIP bool zcip default y diff --git a/networking/wget.c b/networking/wget.c index 5dac2b5..f604828 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -87,7 +87,6 @@ struct globals { #define G (*ptr_to_globals) #define INIT_G() do { \ SET_PTR_TO_GLOBALS(xzalloc(sizeof(G))); \ - IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = 900;) \ } while (0) @@ -944,7 +943,7 @@ int wget_main(int argc UNUSED_PARAM, char **argv) INIT_G(); - IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = 900;) + IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = CONFIG_WGET_DEFAULT_TIMEOUT;) G.proxy_flag = on; /* use proxies if env vars are set */ G.user_agent = Wget; /* User-Agent header field */ -- 1.8.3.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
[PATCH 2/2] wget: add support for connect timeout
Hi, Connect timeout support for wget. Bloatcheck: function old new delta open_socket 38 71 +33 alarm_handler - 31 +31 wget_main 24322447 +15 ftpcmd 127 130 +3 fgets_and_trim89 92 +3 base64enc 63 66 +3 -- (add/remove: 1/0 grow/shrink: 5/0 up/down: 88/0) Total: 88 bytes textdata bss dec hex filename 21739 970 104 22813591d busybox_old 21980 986 104 230705a1e busybox_unstripped - Lauri -- http://www.fastmail.fm - Or how I learned to stop worrying and love email again From 2a33129f02ce0609ba88e3d4ae98b1bed60654f0 Mon Sep 17 00:00:00 2001 From: Lauri Kasanen cur...@operamail.com Date: Tue, 8 Oct 2013 17:11:22 +0300 Subject: [PATCH 2/2] wget: add support for connect timeout Signed-off-by: Lauri Kasanen cur...@operamail.com --- networking/Config.src | 16 +--- networking/wget.c | 18 +- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/networking/Config.src b/networking/Config.src index 0a483ae..987ae26 100644 --- a/networking/Config.src +++ b/networking/Config.src @@ -970,16 +970,18 @@ config FEATURE_WGET_LONG_OPTIONS Support long options for the wget applet. config FEATURE_WGET_TIMEOUT - bool Enable read timeout option -T SEC + bool Enable timeout option -T SEC default y depends on WGET help - Supports network read timeout for wget, so that wget will give - up and timeout when reading network data, through the -T command - line option. Currently only network data read timeout is - supported (i.e., timeout is not applied to the DNS nor TCP - connection initialization). When FEATURE_WGET_LONG_OPTIONS is - also enabled, the --timeout option will work in addition to -T. + Supports network read and connect timeouts for wget, + so that wget will give up and timeout, through the -T + command line option. + + Currently only connect and network data read timeout are + supported (i.e., timeout is not applied to the DNS query). When + FEATURE_WGET_LONG_OPTIONS is also enabled, the --timeout option + will work in addition to -T. config WGET_DEFAULT_TIMEOUT int Default wget timeout diff --git a/networking/wget.c b/networking/wget.c index f604828..5e3533a 100644 --- a/networking/wget.c +++ b/networking/wget.c @@ -72,6 +72,7 @@ struct globals { const char *user_agent; /* User-Agent header field */ #if ENABLE_FEATURE_WGET_TIMEOUT unsigned timeout_seconds; + bool connecting; #endif int output_fd; int o_flags; @@ -198,12 +199,16 @@ static FILE *open_socket(len_and_sockaddr *lsa) { FILE *fp; + IF_FEATURE_WGET_TIMEOUT(G.connecting = 1; alarm(G.timeout_seconds);) + /* glibc 2.4 seems to try seeking on it - ??! */ /* hopefully it understands what ESPIPE means... */ fp = fdopen(xconnect_stream(lsa), r+); if (fp == NULL) bb_perror_msg_and_die(bb_msg_memory_exhausted); + IF_FEATURE_WGET_TIMEOUT(G.connecting = 0;) + return fp; } @@ -907,6 +912,14 @@ However, in real world it was observed that some web servers free(redirected_path); } +#if ENABLE_FEATURE_WGET_TIMEOUT +static void alarm_handler(int sig UNUSED_PARAM) +{ + if (G.connecting) + bb_error_msg_and_die(download timed out); +} +#endif + int wget_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; int wget_main(int argc UNUSED_PARAM, char **argv) { @@ -943,7 +956,10 @@ int wget_main(int argc UNUSED_PARAM, char **argv) INIT_G(); - IF_FEATURE_WGET_TIMEOUT(G.timeout_seconds = CONFIG_WGET_DEFAULT_TIMEOUT;) +#if ENABLE_FEATURE_WGET_TIMEOUT + G.timeout_seconds = CONFIG_WGET_DEFAULT_TIMEOUT; + signal(SIGALRM, alarm_handler); +#endif G.proxy_flag = on; /* use proxies if env vars are set */ G.user_agent = Wget; /* User-Agent header field */ -- 1.8.3.1 ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH 3/3] wall: Temporarily drop privileges when opening files
On Tuesday 08 October 2013 14:55:44 Denys Vlasenko wrote: On Tuesday 08 October 2013 02:02, Ryan Mallon wrote: The wall applet is setuid and currently does no checking of the real user's read access to the message file. This allows the wall applet to be used to display files which are not readable by an unprivileged user. For example: $ wall /etc/shadow $ wall /proc/vmallocinfo Fix this by temporarily dropping privileges before opening the file. Applied all three patches (with small modifications). Thanks! Hi, the crontab patch seems not to be in git yet. git pull Updating aadb485..932e233 Fast-forward archival/libarchive/decompress_bunzip2.c | 26 + archival/libarchive/get_header_ar.c | 35 +++-- archival/tar.c |4 +- console-tools/dumpkmap.c | 37 +++--- console-tools/loadkmap.c | 27 + coreutils/dd.c | 111 - coreutils/touch.c| 39 +-- docs/tcp.txt | 21 +++--- include/applets.src.h|2 - include/libbb.h |4 +- init/init.c | 35 +++-- libbb/human_readable.c |6 ++- libbb/lineedit.c | 66 +++ libbb/unicode.c |5 +-- libbb/xfuncs_printf.c| 12 +++--- miscutils/Config.src |7 miscutils/Kbuild.src |1 - miscutils/man.c |2 +- miscutils/setsid.c | 14 ++- miscutils/wall.c | 25 +++- networking/httpd.c | 39 --- networking/ifconfig.c|2 +- networking/interface.c |2 +- networking/libiproute/iplink.c |2 +- networking/udhcp/dhcpc.c | 49 +++ procps/nmeter.c |3 +- procps/powertop.c|3 +- procps/ps.c |9 ++--- procps/top.c |3 +- scripts/trylink |6 +-- util-linux/fdisk_gpt.c |8 ++-- util-linux/swaponoff.c | 14 +++ 32 files changed, 412 insertions(+), 207 deletions(-) Is there any particular reason we use setfsgid/setfsuid that are Linux-specific and should not be used in programs intended to be portable. I just checked in a older android bionic libc and the calls are not there so sooner or later the android or BSD people will start to complain. I also checked the latest util-linux tar 2.24-rc1 and in wall they do: if (fname) { /* * When we are not root, but suid or sgid, refuse to read files * (e.g. device files) that the user may not have access to. * After all, our invoker can easily do wall file * instead of wall file. */ uid_t uid = getuid(); if (uid (uid != geteuid() || getgid() != getegid())) errx(EXIT_FAILURE, _(will not read %s - use stdin.), fname); if (!freopen(fname, r, stdin)) err(EXIT_FAILURE, _(cannot open %s), fname); } Couldn't we use a similar solution? I also have some troubles making wall working correctly in my xterm: ./busybox wall prova wall: can't open '/dev/:0': No such file or directory ls -la busybox -rwsr-sr-x 1 root root 753004 Oct 8 18:30 busybox but adding this 2 lines fixes it: while ((ut = getutent()) != NULL) { char *line; if (ut-ut_type != USER_PROCESS) continue; + if (ut-ut_line[0] == ':') + continue; line = concat_path_file(/dev, ut-ut_line); xopen_xwrite_close(line, msg); free(line); } Comment in wall of util-linux 2.24-rc1 says: /* Joey Hess reports that use-sessreg in /etc/X11/wdm/ produces ut_line entries like :0, and a write to /dev/:0 fails. */ Cannot say if it is worth to add them to busybox. Ciao, Tito ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
How to contact Philips firmware developers?
Hi, I have an untypical question today. How to contact Philips firmware developers? Why: I bought a Philips TV and there is a small problem with it. It has neither VGA nor DVI inputs, only HDMI (and a few others, such as component video). This is typical for the recent TVs. But, of course, there are still many notebooks and PCs which have DVI and VGA outputs. These outputs have only video signals, no audio. So to accomodate PCs and notebooks, my TV has an additional audio input, labeled Audio in DVI. It's a standard 3.5mm stereo jack. If I connect my PC (using DVI-to-HDMI cable), TV realises this somehow (perhaps by looking at HDMI signal and not seeing audio signals there). Then it plays audio which comes from Audio in DVI, all is well. But my other machine, a notebook, has DisplayPort output. DisplayPort should have audio signal too, but google says my notebook is not capable of sending audio over its DisplayPort output. I tried DisplayPort-to-HDMI cable. Video works, audio doesn't (as I expected), but when I connect audio output from my notebook to Audio in DVI input, it **does not work too**. Stupid TV sees that video input is not DVI! I tried to fool the TV by using two cables: DisplayPort-to-DVI, then DVI-to-HDMI. No luck. Audio coming to Audio in DVI input still **does not work**. Stupid TV *still* sees that video input is not DVI! Apparently what can help here is to override these TV smarts. I want TV to stop guessing is it a DVI signal? and just force it to use Audio in DVI input. Needless to say, nothing like that can be found in Setup menu. I talked to Philips support. I have hard time explaining them that I want to send an email to their firmware developers. They don't understand me, and as soon as they realize I have a *feature request*, not a genuinely broken TV, they shoot me down. I decided to ask here. Any ideas how to contact Philips firmware developers? How would you go about contacting firmware developers in general? I mean, how to penetrate layers of crappy customer support and reach someone who actually knows something technical? -- vda ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] wall: Don't allow reading of files the real user might not have access to
On Tue, Oct 08, 2013 at 10:57:58AM +1100, Ryan Mallon wrote: On 07/10/13 00:13, Denys Vlasenko wrote: On Monday 30 September 2013 00:30, Ryan Mallon wrote: The wall applet is setuid and currently does no checking of the real user's read access to the message file. This allows the wall applet to be used to display files which are not readable by an unprivileged user. For example: $ wall /etc/shadow $ wall /proc/vmallocinfo Fix this issue by introducing the same check as used by the util-linux version of wall: only allow the file argument to be used if the user is root, or the real and effective uid/gids are equal. Note, some files in /proc, etc do have global read permission, but may have different contents if read as root (for example Linux's kptr_restrict feature). User's may still run: $ wall file If they do legitimately have read access to some file. Signed-off-by: Ryan Mallon rmal...@gmail.com --- diff --git a/miscutils/wall.c b/miscutils/wall.c index 762f53b..f0a6cd4 100644 --- a/miscutils/wall.c +++ b/miscutils/wall.c @@ -23,6 +23,20 @@ int wall_main(int argc UNUSED_PARAM, char **argv) struct utmp *ut; char *msg; int fd = argv[1] ? xopen(argv[1], O_RDONLY) : STDIN_FILENO; + uid_t ruid; + + /* + * If we are not root, but are setuid or setgid, then don't allow + * reading of any files the real uid might not have access to. The + * user can do 'wall file' instead of 'wall file' if this disallows + * access to some legimate file. + */ + ruid = getuid(); + if (fd != STDIN_FILENO ruid != 0 + (ruid != geteuid() || getgid() != getegid())) { + bb_error_msg_and_die(will not read %s - use stdin or '%s %s', + argv[1], argv[0], argv[1]); + } msg = xmalloc_read(fd, NULL); if (ENABLE_FEATURE_CLEAN_UP argv[1]) How about the following patch? - --- busybox.3/miscutils/wall.c 2013-07-25 03:48:31.0 +0200 +++ busybox.4/miscutils/wall.c 2013-10-06 15:01:55.0 +0200 @@ -22,8 +34,19 @@ int wall_main(int argc UNUSED_PARAM, cha { struct utmp *ut; char *msg; - int fd = argv[1] ? xopen(argv[1], O_RDONLY) : STDIN_FILENO; + int fd; + fd = STDIN_FILENO; + if (argv[1]) { + /* The applet is setuid. +* Access to the file must be under user's uid/gid. +*/ + setfsuid(getuid()); + setfsgid(getgid()); Also, the setfsuid()/setfsgid() functions are Linux specific and don't appear to be able to return errors to the caller properly. The manpage suggests that they are no longer necessary since the signalling issues they were introduced for in Linux no longer exist. I think it is safe/portable to set setegid()/seteuid(). They are still necessary, but for a different reason: euid/egid apply to the whole process, whereas fsuid/fsgid are thread-local. So in a multithreaded server attempting to access files while enforcing the permissions of a particular user, you may still want fsuid/fsgid. In the case of busybox, I think euid/egid are ok. Rich ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox