Secure Programming Cookbook for C and C++

Errata for Secure Programming Cookbook for C and C++

Submit your own errata for this product.


The errata list is a list of errors and their corrections that were found after the product was released.

The following errata were submitted by our customers and have not yet been approved or disproved by the author or editor. They solely represent the opinion of the customer.


Color Key: Serious Technical Mistake Minor Technical Mistake Language or formatting error Typo Question Note Update



Version Location Description Submitted By Date Submitted
Printed Page 4
5th paragraph

Page 4 appears to be an enumeration of user environmental variables. Two consecutive paragraphs on this page start with "Finally,". In the second instance, the name of the variable appears to be missing.

Anonymous   
Printed Page 4
6th para

"Finally, a special environment variable,, ..." Missing variable name, no?

Robert P. J. Day  Dec 12, 2009 
Printed Page 5.25.3 - p240
SpcEncrypt code example

The data length and buffer length arguments to the first call to CryptEncrypt are swapped around. The buffer length is getting passed in as the data length and the call always fails. if (!CryptEncrypt(hKey, 0, bFinal, 0, pbResult, &dwDataLen, *cbData)) { LocalFree(pbResult); return 0; } *cbData = dwDataLen; return pbResult; should just be: if (!CryptEncrypt(hKey, 0, bFinal, 0, pbResult, cbData, dwDataLen)) { LocalFree(pbResult); return 0; } return pbResult; The second call to CryptEncrypt in this function is correct.

Anonymous   
Other Digital Version 19
2/3rds down the code example

actually, the code printed in the book (at least, as viewed on google books) is correct. what's far worse is that the downloadable tarball ( spc-1.1.tar.gz) contains a typo. in the tarball, the crucial call to setreuid() is, instead, a call to setregid(). happily, this causes an abort() later on when the uid hasn't actually been successfully changed. but there's a very real risk of a user assuming the check is wrong, and deleting it, rather than tracking down the issue. (it took me about an hour to notice the typo.)

Paul Fox  Feb 14, 2014 
Printed Page 28
verbatim code

In the spc_fork() function, the function spc_sanitize_files() shall not be called from this function. Indeed spc_sanitize_files() closes all file descriptors (fd) but when spc_fork() is used by the function spc_popen (given in page 32) the fd of the two pipes (stdin_pipe and stdout_pipe) are also closed which is not good because they are used by the child and parent process. As proove: halt the process before and after spc_sanitize_files() in Linux see /proc/1234/fd where 1234 = getpid(). You'll see 4 pipes fd before and no more fd after. This function for calling ls command is ok (working for me) but for calling an external software to interact with it (read its output and send to it data on its stdin) is not working. This can be fixed quite easily : 1/ pass pipes fd to the spc_sanitize_files(int stdin_pipe[2], int stdout_pipe[2]) 2/ Instead of for (fd = 3; fd < fds; fd++) close(fd); Do not close fd for pipes: for (fd = 3; fd < fds; fd++) { if (fd != stdin_pipe[0] && fd != stdin_pipe[1] && fd != stdout_pipe[0] && fd != stdout_pipe[1]) { close(fd); }} 3/ do not call spc_fork() inside spc_popen() but call spc_sanitize_files(stdin_pipe, stdout_pipe), spc_drop_privileges() and reseed() before calling execve().

Quadrat  Nov 15, 2019 
Printed Page 32
verbatim code

In the spc_popen() why using FILE* and not simply using int file descriptors ? Especially for writing in the pipe the fwrite() uses a buffer and therefore your data is not sent and you have to call fflush() to force them to be sent. Using write() looks better.

Quadrat  Nov 15, 2019 
Printed Page 39
Last paragraph, first two lines.

The first two lines of this paragraph say "..., a user may delete or rename any file in a directory that the user owns, regardless of whether the user owns the file." Ownership has no impact on who can delete/rename files, it is determined by the user's ability to write to the directory. The paragraph should probably say "..., a user may delete or rename any file in a directory that the user can write to, ...". The example below demonstrates this on a Linux machine. 10:02:turing 10% ls -ld dir1 dir2 drwxrwxrwx 2 root root 4096 Feb 22 10:01 dir1 dr-xr-xr-x 2 dwmalone system 4096 Feb 22 10:01 dir2 10:02:turing 11% ls -ld dir1/file dir2/file -rw-r--r-- 1 dwmalone system 0 Feb 22 10:01 dir1/file -rw-r--r-- 1 dwmalone system 0 Feb 22 10:01 dir2/file 10:02:turing 12% m -f dir1/file dir2/file rm: cannot remove `dir2/file': Permission denied 10:03:turing 13% ls -ld dir1/file dir2/file ls: dir1/file: No such file or directory -rw-r--r-- 1 dwmalone system 0 Feb 22 10:01 dir2/file

Anonymous   
Printed Page 40
Paragraph 7

The second paragraph of "The setgid bit" paragraph explains the standard sgid directory behaviour on SysV derived systems. It doesn't explain that BSD derived systems act as if the sgid bit is always set

Anonymous   
Printed Page 44
(pseudo-)Code example at the bottom of the page

The pseudo code calls open(), then spc_restore_privileges(), and then does a perror expecting errono to be unchanged from when open is called. However, the setgroups call in spc_restore_privileges() may clobber errno if it fails. (If any of the other calls fail, then the spc_restore_privileges() calls abort(), so that shouldn't be a problem.)

Anonymous   
Printed Page 90
Bad code example in "Unsigned-to-signed coercion" paragraph

This code is also bad (ISO) C for a resaon not mentioned in the text. C only allows you to add a number to a pointer, if the resulting pointer will be between the start and (one past) the end of the chunk of memory that was allocated to produce that object. See section 6.5.6 of the ISO C99 standard for details.

Anonymous   
Printed Page 102
spc_email_isvalid function

If an address without a domain (e.g. "alice") is passed to the function then the domain validation will go past the end of the string. The first for loop can terminate when *c is NUL (the end of the address is reached). When "if (!*(domain = ++c)) return 0;" is executed, domain and c will point to the byte after the NUL that terminates the address. One way to fix it is to change if (c == address || *(c - 1) == '.') return 0; to if (!*c || c == address || *(c - 1) == '.') return 0;

Anonymous   
Printed Page 111
The for loop in the code given for spc_utf8_isvalid

This code doesn't seem to work. Note the use of the variable nb: nb+1 is added to c at the end of each iteration of the for loop. However, because of the while() which is the last statement of the for loop, nb always has value -1 when the loop ends, thus c is never advanced to the next utf-8 character.

Anonymous   
Printed Page 113
bottom Code block

1) Implementation of spc_fd_set(...) is incorrect. You forgot you need to calculate bit sizes, not bytes. Should be: #define BITS 8 void spc_fd_set(int fd, SPC_FD_SET *fdset) { long *tmp_bits; size_t new_size; if (fd < 0) return; if (fd >= fdset->fds_size * BITS) { new_size = sizeof(long)*((fd+(sizeof(long)*BITS))/(sizeof(long)*BITS)); if (!(tmp_bits = (long *)realloc(fdset->fds_bits, new_size))) return; fdset->fds_bits = tmp_bits; fdset->fds_size = new_size; } fdset->fds_bits[fd / (sizeof(long)*BITS)] |= (1 << (fd % (sizeof(long)*BITS))); } 2) Implementation of spc_fd_clr(...) is identical to spc_fd_set(...). (Copy/Paste issue) Probably want something like this: void spc_fd_clr(int fd, SPC_FD_SET *fdset) { if (fd < 0) return; if (fd >= fdset->fds_size*BITS) return; fdset->fds_bits[fd / (sizeof(long)*BITS)] &= ~(1 << (fd % (sizeof(long)*BITS))); } 3) Should also be made clear this recipe will not work in Microsoft Windows.

Anonymous   
Printed Page 113
bottom Code block

1) Implementation of spc_fd_set(...) is incorrect. You forgot you need to calculate bit sizes, not bytes. Should be: #define BITS 8 void spc_fd_set(int fd, SPC_FD_SET *fdset) { long *tmp_bits; size_t new_size; if (fd < 0) return; if (fd >= fdset->fds_size * BITS) { new_size = sizeof(long)*((fd+(sizeof(long)*BITS))/(sizeof(long)*BITS)); if (!(tmp_bits = (long *)realloc(fdset->fds_bits, new_size))) return; fdset->fds_bits = tmp_bits; fdset->fds_size = new_size; } fdset->fds_bits[fd / (sizeof(long)*BITS)] |= (1 << (fd % (sizeof(long)*BITS))); } 2) Implementation of spc_fd_clr(...) is identical to spc_fd_set(...). (Copy/Paste issue) Probably want something like this: void spc_fd_clr(int fd, SPC_FD_SET *fdset) { if (fd < 0) return; if (fd >= fdset->fds_size*BITS) return; fdset->fds_bits[fd / (sizeof(long)*BITS)] &= ~(1 << (fd % (sizeof(long)*BITS))); } 3) Should also be made clear this recipe will not work in Microsoft Windows.

Anonymous  Jul 24, 2008 
Printed Page 114
Top

Continuing previous submission: 1) spc_fd_isset(...) is incorrect due to lack of bit size calculation. Should be: int spc_fd_isset(int fd, SPC_FD_SET *fdset) { if (fd < 0 || fd >= fdset->fds_size*BITS) return 0; return (fdset->fds_bits[fd / (sizeof(long)*BITS)] & (1 << (fd % (sizeof(long)*BITS)))); } 2) and spc_fd_setsize(...) should probably return number of bits, not bytes. int spc_fd_setsize(SPC_FD_SET *fdset) { return fdset->fds_size*BITS; }

Anonymous   
Printed Page 114
Top

Continuing previous submission: 1) spc_fd_isset(...) is incorrect due to lack of bit size calculation. Should be: int spc_fd_isset(int fd, SPC_FD_SET *fdset) { if (fd < 0 || fd >= fdset->fds_size*BITS) return 0; return (fdset->fds_bits[fd / (sizeof(long)*BITS)] & (1 << (fd % (sizeof(long)*BITS)))); } 2) and spc_fd_setsize(...) should probably return number of bits, not bytes. int spc_fd_setsize(SPC_FD_SET *fdset) { return fdset->fds_size*BITS; }

Anonymous  Jul 24, 2008 
Printed Page 117
End of page

In page 117, where it says... /* When statically allocated */ unsigned char *key[KEYLEN_BYTES]; ...why do you need to create a pointer here? This makes sense if we are creating a list of KEYLEN_BYTES length strings, but that's not the case. Thus, this should be changed to... /* When statically allocated */ unsigned char key[KEYLEN_BYTES];

Alex  Apr 23, 2013 
Printed Page 124
2nd statement in source code

The char array b64table is declared to hold 64 characters but it is assigned 65 characters (including the terminating "null"). In fact, modern C/C++ compilers will fail to compile the code. Therefore, the second statement in the code should be static char b64table[65] = "ABCDEF ... 012356789+/";

Anonymous   
Printed Page 124
Near line 20 in example code

In recipe 4.5, the statement that allocates the buffer to pointers 'p' and 'output' (ie. p = output = (unsigned char *)malloc(....) ) does not allocated a big enough buffer in most cases. It is worth noting that the code in the book and the downloadable code are different when it comes to allocating the buffer but both codes did not allocated a big enough buffer. This bug is insidious because the compiler will not catch the error but it will cause a runtime error. You will probably get a runtime error if you try to free the resource returned by spc_base64_encode() because in most cases the pointer 'p' has advanced pass the end of the buffer that was allocated. Thus when you call free() to free the resource, you are trying to free a part of the memory that does not belong to the program. Even if the program does not crash when you free the resource returned by spc_base64_encode(), you still have a memory leak problem. I compiled the code with VC++ 2005 and run it and it caused runtime error every time I try to free the resource returned by spc_base64_encode(). I suspect on other platforms this bug may cause erratic and hard to debug runtime error.

Anonymous   
Printed, Other Digital Version Page 124
Lines 4 & 9 of the code sample (depending on how you count)

This was reported earlier by Anonymous but I wanted to expand on it. The downloaded code sample looks to be an earlier version of the code than what is printed in the book. The problem in the downloaded code is physical line# 14: toalloc = (len / 3) * 4 + (3 - mod) % 3 + 1; It can be fixed as: toalloc = (len / 3) * 4 + (mod? 5: 1); The version in the book tried to fix it on line 21: p = output = (unsigned char *)malloc(((len / 3) + (mod ? 1 : 0)) * 4 + 1); That's essentially the same fix as above, but if you use that then the "wrap" code doesn't work. So neither example is correct.

Anonymous  Sep 18, 2016 
Printed Page 136
Section 4.10.3, in the function "static void pkcs5_F( ... )".

static void pkcs5_F(unsigned char *p, size_t plen, unsigned char *salt, size_t saltlen, size_t ic, size_t bix, unsigned char *out) { size_t i = 1, j, outlen; unsigned char ulast[PRF_OUT_LEN]; memset(out,0, PRF_OUT_LEN); pkcs5_initial_prf(p, plen, salt, saltlen, bix, ulast, &outlen); while (i++ <= ic) { for (j = 0; j < PRF_OUT_LEN; j++) out[j] ^= ulast[j]; pkcs5_subsequent_prf(p, plen, ulast, PRF_OUT_LEN, ulast, &outlen); } for (j = 0; j < PRF_OUT_LEN; j++) out[j] ^= ulast[j]; } pkcs5_initial is called once, and pkcs_subsequent is called ic times, which is a total of ic + 1 iterations of the PRF applied to the password. The while loop should evaluate to: while(i++ < ic) { ... }

Anonymous   
Printed Page 233
2nd Code block (code for openssl decryption)

In the discussion for using OpenSSL low-level encryption and decryption function, it first states that the encryption and decryption routines are completely symmetric. It then goes on to explain the EVP_EncryptFinal_ex() function, and use it in the code block (bottom of page 232, top of page 233). However, in the example code for decryption on page 233, it does not use EVP_DecryptFinal_ex(), so the code examples are not completely symmetric.

Anonymous   
Printed Page 322
Several entries in table 7-2.

Between the entry for BN_nnmod and BN_mod_exp the descriptions say things like: r = |a mod b| Consider when a = -1 and b = 3, then we would consider a mod b to be either -1 or 2, and so the |a mod b| will be 1 or 2. What BN_nnmod actually returns is: r = a mod b, 0 <= r < b unless |a mod b| is a notation for non-negative remainder that I've never seen before. Also, no Description is given for BN_mod_inverse and the comment for BN_div_word (on page 323) is actually supposed to be the comment for BN_mod_word.

Anonymous   
Printed Page 324
1st paragraph

This paragraph says "no even numbers are primes" - what about 2? The fact that two is prime has lots of important implications for cryptographs, it is just that 2 isn't a very good choice as a large prime number! Maybe the paragraph should say "the only even prime number is 2, and it isn't big enough to be useful as a large prime."

Anonymous   
Printed Page 330
code example in Discussion part of 7.7

The line after the code example says "Be sure to deallocate the BIGNUM objects if you're erasing the last references to them". This should probably refer to BN_free() and BN_clear_free() in Recipe 7.4, as the code doesn't make it clear that it is NULLing out a pointer rather than zeroing out a number. Note that the example code in Recipe 7.5 and 7.6 seems to leak memory for BIGNUMs and Contexts, though I haven't tested to be certain.

Anonymous   
Printed Page 374
second code example, definition of spc_group_getname

Function calls libc function "getgruid(gid)", this should be "getgrgid(gid)" as the former function does not exist.

Anonymous   
Printed Page 375
definition of spc_group_ismember function

The spc_group_ismember function only checks a user's secondary groups and does not check a user's primaty group. For example: % groups system vet wheel www floppy src lj4 unsup images submit % tail -5 mem.c int main(int argc, char **argv) { printf("system %d ", spc_group_ismember("system", "dwmalone")); printf("wheel %d ", spc_group_ismember("wheel", "dwmalone")); return 0; } % ./mem system 0 wheel 1

Anonymous   
Printed Page 382
Table 8-1, third entry

The table says that "10/24" means that "any address starting with 10 will be matched", I'm fairly sure this should say "any address starting 10.0.0. will be matched".

Anonymous   
Printed Page 383-384
Definition of spc_host_init function

The bufsz variable can be used before it is initialised. % gcc -W -Wall -pedantic -ansi -O2 -c 4-init.c 4-init.c: In function `spc_host_init': 4-init.c:79: warning: `bufsz' might be used uninitialized in this function

Anonymous   
Printed Page 389
definition load_wordlist function

The variable offsets uses a size_t to store offsets returned by ftell - it should use longs, or even use off_t and ftello().

Anonymous   
Printed Page 467
5th line from top

The line- bResult = HttpSendRequest(hRequest, lpszHeaders,dwHeadersLenght,lpOptional,dwOptionalLenght); should read: bResult = HttpSendRequest(hRequest, lpszHeaders,dwHeadersLenght,lpszOptional,dwOptionalLenght);

Anonymous   
Printed Page 480
Definition of spc_socket_accept

The check of the return value of accept looks wrong: if (!(new_sock->sd = accept(..)) goto error_exit; Accept usually returns -1 on failure and >=0 on success, so this code does the wrong thing if accept returns 0 or -1.

Anonymous   
Printed Page 483
second paragraph

The text says that FreeBSD does not return the process ID of the sending process, however it seems it does: struct cmsgcred { pid_t cmcred_pid; /* PID of sending process */ uid_t cmcred_uid; /* real UID of sending process */ uid_t cmcred_euid; /* effective UID of sending process */ gid_t cmcred_gid; /* real GID of sending process */ short cmcred_ngroups; /* number or groups */ gid_t cmcred_groups[CMGROUP_MAX]; /* groups */ }; I haven't checked NetBSD.

Anonymous   
Printed Page 518
3rd paragraph

"You will be prompted for a password when running the first command" should say "... second command".

Anonymous   
Printed Page 535
3rd paragraph

The second sentence says (roughly) "SSL without cert verification is no better than exchanging keys in the clear" and the third sentence says that "SSL without cert verification gives security against passive eavesdroppers". However, exchanging keys in the clear is vunerable to passive eavesdroppers. The second sentence should probably be deleted, as it is the incorrect one I think.

Anonymous   
Printed Page 537
last code example

In the definition of spc_x509store_setusekey() if (spc_store->use_key) EVP_PKEY_free(key); spc_store->use_key = key; This looks incorrect. spc_store->use_key is being assigned a dangling pointer. The author probably meant to write this: if (spc_store->use_key) EVP_PKEY_free(spc_store->use_key); spc_store->use_key = key;

Summit Tuladhar  Apr 24, 2014 
Printed Page 540
2nd paragraph, last sentence

"If you do not, any site could present you with Microsoft's certificate, claiming it as their own, and it would successfully verify." This could only happen if the site had stolen the private key matching the public key embedded in the certificate. The whole point of certificates is to securely bind an identity to a public key matching a private key verifiably held by the owner of the identity. This sentence is simply wrong.

Anonymous   
Printed Page 571
second paragraph

This paragraph says that a (0.25,0.75) is 50% easier to guess than a fully entropic source (0.5,0.5), however neither the Shannon Entropy or the avergae number of guesses of these quantities are related by a difference of 50%: > (0.5*ln(0.5) + 0.5*ln(0.5))/ln(0.5) 1 > (0.25*ln(0.25) + 0.75*ln(0.75))/ln(0.5) ~.81127812445913286390 > 1*0.5 + 2*0.5 1.5 > 1*0.75 + 2*0.25 1.25 It looks like you should just drop the "50%" and the last sentence in the paragraph.

Anonymous   
Printed Page 572
First poing in "tips on collecting entropy"

The first point seems to say that postprocessing your data can help ensure that your data has enough bits of entropy, however no postprocessing can increase the entropy associated with some data. It can increase the density of entropy, by squashing the entropy into a shorter string, as pointed out in the text, but the wording of the point is slightly misleading.

Anonymous   
Printed Page 585,587,589
Start of code examples

C++ style comments are started with "/*" rather than "//", seems to be fixed in downloadable code examples. 3rd paragraph (not counting aside); This paragraph says that on systems with a /proc, most of the interesting OS info is in /proc. This isn't really true, except on Linux. On most OSes /proc only has running information processes but not on things like interrupts, network stack stats, ...

Anonymous   
Printed Page 714
Code example at end of page

The variables set by the signal handlers should have type "volatile sig_atomic_t", not type "int", see C99 standard section 7.14.1.

Anonymous