193 lines
7.6 KiB
Text
193 lines
7.6 KiB
Text
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN" [
|
|
<!ENTITY base CDATA "..">
|
|
<!ENTITY date "$Date: 1999-01-21 01:49:15 $">
|
|
<!ENTITY title "Security Do's and Don'ts for Programmers">
|
|
<!ENTITY % includes SYSTEM "../includes.sgml"> %includes;
|
|
]>
|
|
<!-- $Id: programmers.sgml,v 1.2 1999-01-21 01:49:15 jkh Exp $ -->
|
|
|
|
<html>
|
|
&header;
|
|
|
|
|
|
<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 4.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, 3.0 or 4.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>
|
|
|
|
&footer
|
|
</body>
|
|
</html>
|