You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@hyperreal.org on 1998/06/20 13:07:39 UTC
cvs commit: apache-1.3/src/support Makefile.tmpl suexec.c
coar 98/06/20 04:07:39
Modified: src/support Makefile.tmpl suexec.c
Log:
Let's not be unnecessarily cryptic in our logs; it interferes with
debugging problems and doesn't really protect anything.
Revision Changes Path
1.23 +1 -1 apache-1.3/src/support/Makefile.tmpl
Index: Makefile.tmpl
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/support/Makefile.tmpl,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -r1.22 -r1.23
--- Makefile.tmpl 1998/05/29 18:21:15 1.22
+++ Makefile.tmpl 1998/06/20 11:07:38 1.23
@@ -68,4 +68,4 @@
htpasswd.o: htpasswd.c $(INCDIR)/conf.h $(OSDIR)/os.h
logresolve.o: logresolve.c $(INCDIR)/conf.h $(OSDIR)/os.h
rotatelogs.o: rotatelogs.c $(INCDIR)/conf.h $(OSDIR)/os.h
-suexec.o: suexec.c $(INCDIR)/conf.h $(OSDIR)/os.h suexec.h
+suexec.o: suexec.c $(INCDIR)/conf.h $(INCDIR)/ap.h $(OSDIR)/os.h suexec.h
1.40 +13 -5 apache-1.3/src/support/suexec.c
Index: suexec.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/support/suexec.c,v
retrieving revision 1.39
retrieving revision 1.40
diff -u -r1.39 -r1.40
--- suexec.c 1998/06/18 19:06:56 1.39
+++ suexec.c 1998/06/20 11:07:38 1.40
@@ -72,6 +72,7 @@
*/
#include "conf.h"
+#include "ap.h"
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -205,7 +206,7 @@
if ((cleanenv = (char **) calloc(AP_ENVBUF, sizeof(char *))) == NULL) {
- log_err("failed to malloc env mem\n");
+ log_err("failed to malloc memory for environment\n");
exit(120);
}
@@ -258,7 +259,14 @@
*/
prog = argv[0];
if (argc < 4) {
- log_err("too few arguments\n");
+ char msgbuf[2048];
+ int i;
+
+ ap_snprintf(msgbuf, sizeof(msgbuf), "too few (%d) arguments:", argc);
+ for (i = 0; i < argc; i++) {
+ ap_snprintf(msgbuf, sizeof(msgbuf), "%s [%s]", msgbuf, argv[i]);
+ }
+ log_err("%s\n", msgbuf);
exit(101);
}
target_uname = argv[1];
@@ -283,12 +291,12 @@
#ifdef _OSD_POSIX
/* User name comparisons are case insensitive on BS2000/OSD */
if (strcasecmp(HTTPD_USER, pw->pw_name)) {
- log_err("user mismatch (%s)\n", pw->pw_name);
+ log_err("user mismatch (%s instead of %s)\n", pw->pw_name, HTTPD_USER);
exit(103);
}
#else /*_OSD_POSIX*/
if (strcmp(HTTPD_USER, pw->pw_name)) {
- log_err("user mismatch (%s)\n", pw->pw_name);
+ log_err("user mismatch (%s instead of %s)\n", pw->pw_name, HTTPD_USER);
exit(103);
}
#endif /*_OSD_POSIX*/
@@ -350,7 +358,7 @@
* Log the transaction here to be sure we have an open log
* before we setuid().
*/
- log_err("uid: (%s/%s) gid: (%s/%s) %s\n",
+ log_err("uid: (%s/%s) gid: (%s/%s) cmd: %s\n",
target_uname, actual_uname,
target_gname, actual_gname,
cmd);
Re: cvs commit: apache-1.3/src/support Makefile.tmpl suexec.c
Posted by Marc Slemko <ma...@worldgate.com>.
This still needs fixing.
On Sat, 20 Jun 1998, Rodent of Unusual Size wrote:
> Marc Slemko wrote:
> >
> > Erm... I don't think this does what you want it to do.
>
> It did in my testing. What in particular do you see it doing
> wrong? Is ap_snprintf() not safe for using the output buffer
> as an input parameter?
>
> #ken P-)}
>
> Ken Coar <http://Web.Golux.Com/coar/>
> Apache Group member <http://www.apache.org/>
> "Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
>
Re: cvs commit: apache-1.3/src/support Makefile.tmpl suexec.c
Posted by Marc Slemko <ma...@worldgate.com>.
On Sat, 20 Jun 1998, Rodent of Unusual Size wrote:
> Marc Slemko wrote:
> >
> > Erm... I don't think this does what you want it to do.
>
> It did in my testing. What in particular do you see it doing
> wrong? Is ap_snprintf() not safe for using the output buffer
> as an input parameter?
In general, most functions dealing with strings in this way aren't safe
using overlapping input and output. Unless a function (eg. bcopy)
explicitly says it can do overlapping copies, you need to assume (eg.
memcpy) that it doesn't.
For example, try running:
int main () {
char buf[4096];
strcpy(buf, "this is a test");
ap_snprintf(buf, sizeof(buf), "xxxx yyyyy %s", buf);
printf("%s\n", buf);
}
It gives:
xxxx yyyyy xxxx yyyyy xxx
The only reason the particular code you use works is because the first
thing copied happens to be the data that will be overwritten. However,
changes to either to ap_snprintf code (say for some resaon it decided to
copy certain things in reverse order) or a tiny change to the format can
break it in non-obvious ways.
It is also horribly inefficient, repeatedly copying the same data over and
over in a loop, but that isn't an overly huge issue for this particular
error message.
Re: cvs commit: apache-1.3/src/support Makefile.tmpl suexec.c
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Marc Slemko wrote:
>
> Erm... I don't think this does what you want it to do.
It did in my testing. What in particular do you see it doing
wrong? Is ap_snprintf() not safe for using the output buffer
as an input parameter?
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: cvs commit: apache-1.3/src/support Makefile.tmpl suexec.c
Posted by Marc Slemko <ma...@worldgate.com>.
On 20 Jun 1998 coar@hyperreal.org wrote:
> - log_err("too few arguments\n");
> + char msgbuf[2048];
> + int i;
> +
> + ap_snprintf(msgbuf, sizeof(msgbuf), "too few (%d) arguments:", argc);
> + for (i = 0; i < argc; i++) {
> + ap_snprintf(msgbuf, sizeof(msgbuf), "%s [%s]", msgbuf, argv[i]);
> + }
> + log_err("%s\n", msgbuf);
Erm... I don't think this does what you want it to do.