205 lines
8.6 KiB
HTML
205 lines
8.6 KiB
HTML
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
|
||
<HTML>
|
||
<HEAD>
|
||
<TITLE>Security Do's and Don'ts for Programmers</TITLE>
|
||
</HEAD>
|
||
<BODY BGCOLOR="#FFFFFF" TEXT="#000000" ALINK="#FFCC33"><IMG SRC="../gifs/bar.gif" ALT="Navigation Bar" HEIGHT="33" WIDTH="565" BORDER="0" USEMAP="#bar">
|
||
<H1 ALIGN="LEFT"><FONT COLOR="#660000">Security Do's and Don'ts for Programmers</FONT></H1><BR CLEAR="ALL">
|
||
|
||
|
||
<MAP NAME="bar">
|
||
<AREA SHAPE="RECT" COORDS="1,1,111,31" HREF="../index.html" ALT="Top">
|
||
<AREA SHAPE="RECT" COORDS="112,11,196,31" HREF="../ports/index.html" ALT="Applications">
|
||
<AREA SHAPE="RECT" COORDS="196,12,257,33" HREF="../support.html" ALT="Support">
|
||
<AREA SHAPE="RECT" COORDS="256,12,365,33" HREF="../docs.html" ALT="Documentation">
|
||
<AREA SHAPE="RECT" COORDS="366,13,424,32" HREF="../commercial.html" ALT="Vendors">
|
||
<AREA SHAPE="RECT" COORDS="425,16,475,32" HREF="../search.html" ALT="Search">
|
||
<AREA SHAPE="RECT" COORDS="477,16,516,33" HREF="../index-site.html" ALT="Index">
|
||
<AREA SHAPE="RECT" COORDS="516,15,562,33" HREF="../index.html" ALT="Top">
|
||
<AREA SHAPE="RECT" COORDS="0,0,564,32" HREF="../index.html" ALT="Top">
|
||
</MAP>
|
||
|
||
|
||
<P></P><UL>
|
||
<LI><A NAME="#rule1"></A>Never trust any source of input, i.e. command line
|
||
arguments, environment variables, configuration files, incoming UDP packets,
|
||
hostname lookups, function arguments, etc. If the length or contents of
|
||
the data received is at all subject to outside control then the program
|
||
or function should watch for this when copying it around. Specific
|
||
security issues to watch for in this area are:
|
||
|
||
<P></P><UL>
|
||
<LI><A NAME="#rule1_1"></A>strcpy() and sprintf() calls from
|
||
unbounded data. Use strncpy() and snprintf() when the length is known
|
||
(or implement some other form of bounds-checking when it's not).
|
||
In fact, never use gets(3) or sprintf(3), period.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_1"></A>strncpy() and strncat() calls. Be sure
|
||
you understand how these work\! strncpy() might not append a terminating
|
||
\\0 while strncat() always adds the \\0.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_2"></A>Watch for strvis(3) and getenv(3) abuse.
|
||
strvis() is easy to get the destination string wrong for, and getenv()
|
||
can return strings much longer than the user might expect - they are
|
||
one of the key ways an attack is often made on a program, causing it
|
||
to overwrite stack or variables by setting its environment variables
|
||
to unexpected values. If your program reads environment variables,
|
||
be paranoid!
|
||
|
||
<P></P></LI>
|
||
<LI>Every time you see an open(2) or stat(2) call, ask yourself, "What
|
||
if it's a symbolic link?"
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_3"></A>All uses of mktemp(), tempnam(), mkstemp(),
|
||
etc.; make sure that they use mkstemp() instead. Also look for races in
|
||
/tmp in general, being aware that there are very few things can be atomic
|
||
in /tmp:
|
||
<UL>
|
||
<LI>Creating a directory. This will either succeed or fail.
|
||
</LI>
|
||
<LI>Opening a file O_CREAT | O_EXCL
|
||
</LI>
|
||
</UL>
|
||
mkstemp(3) properly handles this for you, so all temp files should
|
||
use mkstemp to guarantee there's no race and that the permissions
|
||
are right.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_4"></A>If an attacker can force packets to go/come
|
||
from another arbitrary system then that hacker has complete control
|
||
over the data that we get and *NONE* of it should be trusted.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_5"></A>Understand the differences between uid,
|
||
euid and svuid in 2.1 and 2.2. We sure don't. [XXX but we should find out
|
||
and fill this in after talking to Bruce]
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_6"></A>Never trust that a config file is correctly
|
||
formatted or that it was generated by the appropriate utility. If there
|
||
is some chance for being sneaky, then some twisted cracker will try
|
||
to be sneaky: Don't trust user input like terminal names or language
|
||
strings to be free of '/' or ../../... embedded if there is any chance
|
||
that they can be used in a path name. Don't trust *ANY* paths supplied
|
||
by the user when you are running setuid root.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_7"></A>Look for holes or weaknesses in how data
|
||
is stored. All temp files should be 600 permission.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_8"></A>Don't just grep for the usual suspects
|
||
in programs which run at elevated privs. Look line by line for possible
|
||
overflows in these cases since there are a lot more ways than strcpy()
|
||
and friends to cause buffer overflows.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule1_9"></A>Just because you drop privs somewhere doesn't
|
||
necessarily mean that no exploit is possible. The attacker may put the
|
||
necessary code on the stack to regain them before execing /bin/sh.
|
||
</LI>
|
||
</UL>
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule2"></A>Do uid management. So drop privs as soon as possible,
|
||
and really drop them. Switching between euid and uid is not enough. Use
|
||
setuid() when you can.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule3"></A>Never display configuration file contents on errors.
|
||
A line number and perhaps position count is enough. This is true for all
|
||
libs and for any SUID/SGID program.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4"></A>Tips for those reviewing existing code for security
|
||
problems:
|
||
|
||
<P></P><UL>
|
||
<LI><A NAME="#rule4_1"></A>If you're unsure of your security fixes, send them
|
||
to a reviewer with whom you've already made arrangements for a second glance
|
||
over. Don't commit code you're not sure of since breaking something in
|
||
the name of securing it is rather embarrassing. :)
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4_2"></A>Those without CVS commit privileges should make
|
||
sure that a reviewer with such privileges is among the last to review the
|
||
changes. That person will both review and incorporate the final version
|
||
you would like to have go into the tree.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4_3"></A>When sending changes around for review, always
|
||
use context or unidiff format diffs which may be easily fed to patch(1).
|
||
Do not simply send whole files! Diffs are much easier to read and apply to
|
||
local sources (especially those in which multiple, simultaneous changes
|
||
may be taking place). All changes should be relative to 3.0-current
|
||
just so we can all be working from a common base, unless there is strong
|
||
reason in a specific instance to do otherwise.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4_4"></A>Always directly test your changes (e.g. build and
|
||
run the affected module(s)) before sending them to a reviewer; no one likes
|
||
being sent obviously broken stuff for review, and it just makes it appear
|
||
as though the submitter didn't even really look at what he was
|
||
doing (which is hardly confidence-building). If you need accounts
|
||
on a 2.1, 2.2 or 3.0 machine in order to do proper testing, just
|
||
ask - the project has such resources available for just such
|
||
purposes.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4_5"></A>For committers: Be sure to retrofit -current
|
||
patches into the 2.2 and 2.1 branches as appropriate.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule4_6"></A>Do not needlessly rewrite code to suit
|
||
your style/tastes - it only makes the reviewer's job needlessly more
|
||
difficult. Do so only if there are clear technical reasons for it.
|
||
</LI>
|
||
</UL>
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule5"></A>Look out for programs doing complex things in
|
||
signal handlers. Many routines in the various libraries are not
|
||
sufficiently reentrant to make this safe.
|
||
|
||
<P></P></LI>
|
||
<LI><A NAME="#rule6"></A>Pay special attention to realloc() usage - more
|
||
often than not, it's not done correctly.
|
||
|
||
<P></P></LI>
|
||
<LI>When using fixed-size buffers, use sizeof() to prevent lossage when
|
||
a buffer size is changed but the code which uses it isn't. For example:
|
||
<LISTING> char buf[1024];
|
||
struct foo { ... };
|
||
...
|
||
BAD:
|
||
xxx(buf, 1024)
|
||
xxx(yyy, sizeof(struct foo))
|
||
GOOD:
|
||
xxx(buf, sizeof(buf))
|
||
xxx(yyy, sizeof(yyy))</LISTING>
|
||
|
||
Be careful though with sizeof of pointers when you really want the size
|
||
of where it points to\!
|
||
<P></P></LI>
|
||
<LI>Every time you see "char foo[###]", check every usage of foo to
|
||
make sure it can't be overflowed. If you can't avoid overflow
|
||
(and cases of this have been seen) then at least malloc the buffer
|
||
so you can't walk on the stack.
|
||
|
||
<P></P></LI>
|
||
<LI>Always close file descriptors as soon as you can -- this makes it
|
||
more likely that the stdio buffer contents will be discarded. In
|
||
library routines, always set any file descriptors that you open to
|
||
close-on-exec.
|
||
|
||
<P></P></LI>
|
||
</UL>
|
||
|
||
<HR NOSHADE>
|
||
<ADDRESS><A HREF="../mailto.html">questions@FreeBSD.ORG</A><BR>
|
||
Copyright <20> 1995-1998 FreeBSD Inc.
|
||
All rights reserved.<BR>$Date: 1998-06-19 09:46:50 $</ADDRESS> </BODY>
|
||
</HTML>
|