1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
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.
|
