summaryrefslogtreecommitdiffstats
path: root/doc/securecoding.txt
diff options
context:
space:
mode:
authorDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-15 17:36:47 +0000
committerDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-15 17:36:47 +0000
commit0441d265f2bb9da249c7abf333f0f771fadb4ab5 (patch)
tree3f3789daa2f6db22da6e55e92bee0062a7d613fe /doc/securecoding.txt
parentInitial commit. (diff)
downloaddovecot-0441d265f2bb9da249c7abf333f0f771fadb4ab5.tar.xz
dovecot-0441d265f2bb9da249c7abf333f0f771fadb4ab5.zip
Adding upstream version 1:2.3.21+dfsg1.upstream/1%2.3.21+dfsg1
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'doc/securecoding.txt')
-rw-r--r--doc/securecoding.txt142
1 files changed, 142 insertions, 0 deletions
diff --git a/doc/securecoding.txt b/doc/securecoding.txt
new file mode 100644
index 0000000..72489ae
--- /dev/null
+++ b/doc/securecoding.txt
@@ -0,0 +1,142 @@
+Simplicity provides security. The more you have to remember to maintain
+security the easier it is to forget something.
+
+
+Use Multiple Layers of Security
+-------------------------------
+
+Input validation is useful to prevent clients from taking too much server
+resources. Add the restrictions only where it's useful. For example a
+simple "maximum line length" will limit the length of pretty much all
+possible client input.
+
+Don't rely on input validation. Maybe you missed something. Maybe someone
+calls your function somewhere else where you didn't originally intend it.
+Maybe someone makes the input validation less restrictive for some reason.
+Point is, it's not an excuse to cause a security hole just because input
+wasn't what you expected it to be.
+
+Don't trust memory. If code somewhere overflowed a buffer, don't make it
+easier to exploit it. For example if you have code:
+
+ static char staticbuf[100];
+ ..
+ char stackbuf[100];
+ strcpy(stackbuf, staticbuf);
+
+Just because staticbuf was declared as [100], it doesn't mean it couldn't
+contain more data. Overflowing static buffers can't be directly exploited,
+but the strcpy() overflowing stackbuf makes it possible. Always copy data
+with bounds checking.
+
+
+Prevent Buffer Overflows
+------------------------
+
+Avoid writing to buffers directly. Write everything through buffer API
+(lib/buffer.h) which guarantees protection against buffer overflows.
+There are various safe string APIs as well (lib/str.h, lib/strfuncs.h).
+Dovecot also provides a type safe array API (lib/array.h).
+
+If you do write to buffers directly, mark the code with /* @UNSAFE */
+unless it's _obviously_ safe. Only obviously safe code is calling a
+function with (buffer, sizeof(buffer)) parameters. If you do _any_
+calculations with buffer size, mark it unsafe.
+
+Use const with buffers whenever you can. It guarantees that you can't
+accidentally modify it.
+
+Use "char *" only for NUL-terminated strings. Use "unsigned char *"
+if it's not guaranteed to be NUL-terminated.
+
+
+Avoid free()
+------------
+
+Accessing freed memory is the most difficult problem to solve with C code.
+Only real solution is to use garbage collector, but it's not possible to
+write a portable GC without radical changes in how you write code.
+
+There are a few ways to avoid most free() calls however: data stack and
+memory pools.
+
+Data stack works in somewhat similar way to C's control stack. alloca() is
+quite near to what it does, but there's one major difference: Stack frames
+are explicitly defined, so functions can return values allocated from data
+stack. t_strdup_printf() call is an excellent example of why this is
+useful. Rather than creating some arbitrary sized buffer and using
+snprintf() which may truncate the value, you can just use t_strdup_printf()
+without worrying about buffer sizes being large enough.
+
+Try to keep the allocations from data stack small, since the data stack's
+highest memory usage size is kept for the rest of the process's lifetime.
+The initial data stack size is 32kB and it should be enough in normal use.
+See lib/data-stack.h.
+
+Memory pools are useful when you have to construct an object from multiple
+pieces and you can free it all at once. Actually Dovecot's Memory Pool API
+is just an abstract class for allocating memory. There's system_pool for
+allocating memory with calloc(), realloc() and free() and you can create a
+pool to allocate memory from data stack. If your function needs to allocate
+memory for multiple objects, you may want to take struct pool as parameter
+to allow caller to specify where the memory is allocated from.
+See lib/mempool.h
+
+
+Deinitialize safely
+-------------------
+
+Whenever you free a pointer, set it to NULL. That way if you accidentally
+try to free it again, it's less likely to cause a security hole. Dovecot
+does this automatically with most of its free() calls, but you should also
+make it a habit of making all your _destroy() functions take a
+pointer-to-pointer parameter which you set to NULL.
+
+Don't Keep Secrets
+------------------
+
+We don't do anything special to protect ourself against read access buffer
+overflows, so don't store anything sensitive in memory. We use multiple
+processes to protect sensitive information between users.
+
+When dealing with passwords and such, erase them from memory after you
+don't need it anymore. Note that such memset() may be optimized away by
+compiler, use safe_memset().
+
+
+Use GCC Extensions
+------------------
+
+GCC makes it easy to catch some potential errors:
+
+Format string vulnerabilities can be prevented by marking all functions
+using format strings with __attr_format__() and __attr_format_arg__()
+macros and using -Wformat=2 GCC option.
+
+-W option checks that you don't compare signed and unsigned variables.
+
+I hope GCC will later emit a warning whenever there's potential integer
+truncation. -Wconversion kind of does that, but it's not really meant for
+it and it gives too many other useless warnings.
+
+
+Use union Safely
+----------------
+
+Suppose there was code:
+
+union {
+ unsigned int number;
+ char *str;
+} u;
+
+If it was possible for user to set number arbitrarily, but access the union
+as string it'd be possible to read or write arbitrary memory locations.
+
+There's two ways to handle this. First would be to avoid union entirely and
+use a struct instead. You don't really need the extra few bytes of memory
+that union saves.
+
+Another way is to access the union only through macro that verifies that
+you're accessing it correctly. See IMAP_ARG_*() macros in
+lib-imap/imap-parser.h.