The arpwatch can (rightfully) be considered a security software. Its use of mktemp, while safe can still be eliminated. The attached patch is rather simple and can be dropped into the patches subdirectory of the port... It also makes the report function a little bit faster. I'm fairly certain the report() function can use vfork() instead of fork, but I'm not 100% sure :) -- it is right next to the area I'm patching -- please review...
Mikhail Teterin wrote: > >Synopsis: net/arpwatch patched to use tmpfile() instead of mktemp() Why not mkstemp()? According to the tmpfile() man page, this function may not be portable across platforms. So, probably, you have better chances that your patches will be accepted by the authors of arpwatch if you will use mkstemp() instead. -Maxim
On 6 Jun, Maxim Sobolev wrote: = Mikhail Teterin wrote: = = > >Synopsis: net/arpwatch patched to use tmpfile() instead of mktemp() = = Why not mkstemp()? According to the tmpfile() man page, this function = may not be portable across platforms. So, probably, you have better = chances that your patches will be accepted by the authors of arpwatch = if you will use mkstemp() instead. Because I need FILE * instead of a file descriptor and because I think the name tmpfile is easier to understand then mkstemp. Also tmpfile will obey TMPDIR environment variable and it does not need the character template. I think, tmpfile is a better choice, although tmpfile(3) disagrees. All of this is hardly a big problem, but I believe the file looks nicer and easier to read this way. As far as the standards go, from tmpfile(3): STANDARDS The tmpfile() and tmpnam() functions conform to ISO 9899: 1990 (``ISO C''). So compatability should not be a problem... Yours, -mi
On Tue, Jun 06, 2000 at 09:40:02AM -0700, mi@privatelabs.com wrote: > Because I need FILE * instead of a file descriptor and because I think > the name tmpfile is easier to understand then mkstemp. int fd = mkstemp( blah-blah ); FILE *fp = fdopen( fd, mode ); And from tmpfile(3): BUGS: These interfaces are provided for System V and ANSI compatibility only. The mkstemp(3) interface is strongly preferred. [...] Then go on and read about the race conditions etc.. use mkstemp(), that's what it's there for. -aDe -- Ade Lovett, Austin, TX. ade@FreeBSD.org FreeBSD: The Power to Serve http://www.FreeBSD.org/
On 6 Jun, Ade Lovett wrote: = On Tue, Jun 06, 2000 at 09:40:02AM -0700, mi@privatelabs.com wrote: = > Because I need FILE * instead of a file descriptor and because I = > think the name tmpfile is easier to understand then mkstemp. = = int fd = mkstemp( blah-blah ); = FILE *fp = fdopen( fd, mode ); Yes, thanks for pointing out the obvious. I believe, it is also obvious that ``fp = tmpfile()'' is MUCH shorter and cleaner -- it does not even need the ``blah-blah'', mind you, which has to be a writable character string, and usually there is a strcpy involved there too. Please, tone down your responses. = And from tmpfile(3): = = BUGS: = These interfaces are provided for System V and ANSI compatibility = only. The mkstemp(3) interface is strongly preferred. = [...] = = Then go on and read about the race conditions etc.. use mkstemp(), = that's what it's there for. The fact that I happen to disagree with the man-page does not mean that I did not read it. I did. FreeBSD does not need to care: This implementation does not have these flaws, but portable software cannot depend on that. In particular, the tmpfile() interface should not be used in software expected to be used on other systems if there is any possibility that the user does not wish the temporary file to be publicly readable and writable. If other OSes have the poorly implemented tmpfile(), IMHO, it is their problem. My goal was to improve the BSD port of arpwatch. The software maintainers received the courtesy copy. -mi
On Tue, Jun 06, 2000 at 01:09:35PM -0400, mi@privatelabs.com wrote: > Yes, thanks for pointing out the obvious. I believe, it is also obvious > that ``fp = tmpfile()'' is MUCH shorter and cleaner You forgot ".. and potentially susceptible to a number of security issues which may capable of causing the program, and possibly the system, to be compromised." We're trying to get rid of security issues in ports, not add them in. > The fact that I happen to disagree with the man-page does not mean that > I did not read it. I did. FreeBSD does not need to care: Irrelevant. There is a well-defined, secure, interface for creating temporary files. It's called mkstemp(). Use it. The patch as it stands should absolutely not go into the tree, unless y'all just want the port marked FORBIDDEN= "bungled security patch" -aDe -- Ade Lovett, Austin, TX.
On 6 Jun, Ade Lovett wrote: = On Tue, Jun 06, 2000 at 01:09:35PM -0400, mi@privatelabs.com wrote: = > Yes, thanks for pointing out the obvious. I believe, it is also = > obvious that ``fp = tmpfile()'' is MUCH shorter and cleaner = = You forgot ".. and potentially susceptible to a number of security = issues which may capable of causing the program, and possibly the = system, to be compromised." On FreeBSD (and OpenBSD and NetBSD) this is NOT TRUE, and we all know it. = We're trying to get rid of security issues in ports, not add them in. My patch removes a potential security issue in the BSD port of the arpwatch software. Please proof otherwise. = > The fact that I happen to disagree with the man-page does not mean = > that I did not read it. I did. FreeBSD does not need to care: = = Irrelevant. There is a well-defined, secure, interface for creating = temporary files. It's called mkstemp(). Use it. tmpfile() is just as well defined and, on FreeBSD, secure. I also happened to like it better then mkstemp(). = The patch as it stands should absolutely not go into the tree, unless = y'all just want the port marked FORBIDDEN= "bungled security patch" It is sad, that you let your emotions blind you. If there will be someone to knock some sense into you, by, for example, overriding the authority you remind "us'all" about, I'll certainly applaud that person. -mi
On Tue, Jun 06, 2000 at 01:52:48PM -0400, mi@privatelabs.com wrote: > On FreeBSD (and OpenBSD and NetBSD) this is NOT TRUE, and we all know > it. Irrelevant. You're assuming that the code reflects the reality in the manual page. There is an explicit reference to using mkstemp() in the tmpfile() manual page. The correct function to use is left as an exercise to the interested reader. need to understand this. > My patch removes a potential security issue in the BSD port of the > arpwatch software. Please proof otherwise. Your patch replaces a known security issue with a possible security issue, whereas it could be trivially rewritten to remove the security issue. > tmpfile() is just as well defined and, on FreeBSD, secure. I also > happened to like it better then mkstemp(). > > It is sad, that you let your emotions blind you. If there will be > someone to knock some sense into you, by, for example, overriding the > authority you remind "us'all" about, I'll certainly applaud that person. Ad hominem attacks are rarely useful. Yours has been noted for future reference. -aDe -- Ade Lovett, Austin, TX.
On 6 Jun, Ade Lovett wrote: = On Tue, Jun 06, 2000 at 01:52:48PM -0400, mi@privatelabs.com wrote: = > On FreeBSD (and OpenBSD and NetBSD) this is NOT TRUE, and we all = > know it. = = Irrelevant. You're assuming that the code reflects the reality in the = manual page. There is an explicit reference to using mkstemp() in the = tmpfile() manual page. So, you suggest I trust one part of the man page, but not the other? mkstemp can also be implemented poorly for that matter. = > My patch removes a potential security issue in the BSD port of the = > arpwatch software. Please proof otherwise. = = Your patch replaces a known security issue with a possible security = issue, whereas it could be trivially rewritten to remove the security = issue. It could be. But the way I wrote it, it is perfectly fine for all of the OSes involved. I'm afraid, you only jumped to this discussion to "teach me" to use fdopen (you are welcome to classify this "attack" any way you want). You do not seem to care about the security/tripwire patch I submitted recently, for example -- in your not too humble opionion it suffers the same flaws. = > tmpfile() is just as well defined and, on FreeBSD, secure. I also = > happened to like it better then mkstemp(). = > = > It is sad, that you let your emotions blind you. If there will be = > someone to knock some sense into you, by, for example, overriding = > the authority you remind "us'all" about, I'll certainly applaud that = > person. = = Ad hominem attacks are rarely useful. Yours has been noted for future = reference. Yeah, yeah... I'm sorry, but this will probably be my last response on this subject. -mi
State Changed From-To: open->closed Another patch committed. Anyway thanks for reporting and please in the future try to be more cooperative and keep your ego under control.
It looks like this bug report has already been closed out but I thought I'd let everyone know that the latest release of arpwatch: ftp://ftp.ee.lbl.gov/arpwatch.tar.gz uses mkstemp(). Craig