Errata
jm December 3rd, 2006
Below is all currently identified errata. Some issues will be explained in more detail in the blog under the Errata Category. Were planning on leaving this page open for comments so people can post additional errata.
| Ch# | Pages | Type | Credit | Description |
|---|---|---|---|---|
| 2 | 27 | typo | Meder Kydyraliev | First paragraph, Line 4: monkey market accounts should read money market accounts |
| 2 | 41 | typo | Rhys | Last paragraph, last sentence reads "Further, asymmetric encryption" but it should read "Further, symmetric encryption" |
| 3 | 87 | typo | mjf | Second paragraph, Line 3: "However, you should aware" should read "However, you should be aware" |
| 5 | 177 | typo | mjf | First paragraph, Line 4: "Remember that authenticated()" should read "Remember that authenticated". The parenthesis indicate that authenticated is a function, where its actually a local variable. |
| 5 | 188 | typo | Willem De Groef | First inline code block, Line 5: movb $0�0b should read movb $0�0b, %al |
| 5 | 189 | typo | mark | Missing label in the second inline code block |
| 6 | 252 | tech | Dominique Brezinski | Line 6 says (MAX_LEN - 1) is 256, but it is actually 255 |
| 6 | 285 | typo | sp | Listing 6-32, Line 10: if ((ntohl(n->sequence_number) <= g_last_sequence number) should read if (ntohl(n->sequence_number) <= g_last_sequence_number) |
| 6 | 286 | tech | Dominique Brezinski | Listing 6-33, Line 10: if (!memcmp(a, b, sizeof(a))) should read if (!memcmp(a, b, sizeof(*a))) . See Doms analysis below. |
| 7 | 318 | tech | jm | First inline code, Line 5: bloblen = ntohl(blob); should read bloblen = read_int(blob); |
| 7 | 352 | typo | tgo | Listing 7-30, Page 2, Line 21: if(nl){ should read if(nl) |
| 7 | 352 | tech | ilja | Listing 7-30, Function read_line(): ilja noticed a vulnerability in this function that wasnt intended as part of the example. strchr() is called on a potentially unterminated stack buffer, so if the byte 0�0a is found before the null-termination byte 0�00 on the stack, memory will be unintentionally modified. This could lead to an exploitable condition or information disclosure. The string should be null terminated after the call to read(), with code like the following: data[sizeof(data)-1]=\0; |
| 7 | 363 | typo | jm | Listing 7-33, Page 2, Line 24: sizeof(struct cipher) < 0) should read sizeof(struct cipher)) < 0) |
| 8 | 428 | typo | ilja | Listing 8-19, Line 8: poen() should read popen() |
| 8 | 434 | tech | mark | Listing 8-21, Line 12: perform_query(buffer) should read perform_query(buf) |
| 8 | 435 | typo | ilja | Second inline code segment reads $input_data =~ s/[^A-Za-z0-9]/g; should read $input_data =~ s/[^A-Za-z0-9]//g; |
| 9 | 482 | typo | ilja | Last paragraph (pg 482), Line 4: The privileged sendmail group is smmsp, not smssp |
| 9 | 525 | typo | jm | First inline code segment, Line 5: fchmod(fd, 644); should read fchmod(fd, 0644); |
| 10 | 600 | typo | tgo | First inline code segment, Line 12; return env->value); should read return (env->value); |
| 10 | 600 | typo | tgo | Second inline code segment, Line 5; intlength = should read int length = |
| 11 | 625 - 684 | typo | ilja | Left-side page header should read "Chapter 11Windows 1: Objects and the File System" instead of "Chapter 11Objects and the File System" |
| 11 | 642 | typo | jm |
Restricted TokensF should read Restricted Tokens |
| 12 | 730 | typo | Ryan Smith | First sentence: properinitialization should be proper initialization |
| 13 | 805 | typo | tgo | Inline code segment, Line 6; if(read_packet_header(sock, &header)) < 0) should read if(read_packet_header(sock, &header) < 0) |
| 13 | 809 | typo | Ryan Smith | (Last paragraph, first sentence): you should use your scoreboard, not user it |
| 16 | 932 | typo | ilja | Line 8 in the inline code: The automatic variable string_length should be domain_length |
| 16 | 986 | typo | jm | Second paragraph, Line 2: "It takes a request from a user, tough function" should read "It takes a request from a user, through a function" |
| 17 | 1009 | typo | Rhys | First paragraph, last sentence: "and HTTP request" should read "an HTTP request" |
| Ch# | Pages | Type | Credit | Description |
|---|---|---|---|---|
| 2 | 57 | layout | justin | Mislabeled diagram |
| 6 | 224 | tech | jm | "Type coercion" is synonymous with "implicit type conversion," not both explicit and implicit type conversion. Additional info here. |
| 6 | 273 | tech | rCs | The behavior of right shift of a signed variable is implementation-defined, however the explanation on page 273 implies that arithmetic right shift is part of the standard. More information is available here. |
| 7 | 319 | tech | jm | First inline code, Line 9: length = ntohl(&pkt[1]); should read length = read_int(&pkt[1]); |
| 7 | 337 | tech | tgo | Listing 7-23, Line 7: calloc(strlen(string+1,sizeof(string)))) should read calloc(strlen(string)+1, 1))) Line 11: switch(*cp){ should read switch(*string){ Line 14: *dest++ = \; should read *dest++ = \\; Line 20 and 23: case \n: and default: should both be followed by the line escape = 0; Line 24: *string = *dest++; should read *dest++ = *string; |
| 7 | 351 | typo | tgo | Listing 7-30, Line 11: memcpy(buffer->data + buffer->used, n); should read memcpy(buffer->data + buffer->used, data, n); |
| 9 | 482, 484 | typo | ilja | Last paragraph (pg 482), Line 4; and first paragraph (pg 484), Line 2: The privileged sendmail group is smmsp, not smssp |
| 9 | 517 | tech | rCs | Figure 9-5: The diagram implies that the link count attribute of a file inode is updated when that file is the target of a symbolic link. This is incorrect, as the link count only reflects the number of hard links that exist for the file. More information here. |
| 10 | 609 | typo | Ryan Smith | Second paragraph, second sentence: a file that ws parsed should read a file that was parsed |
| 10 | 613 | typo | Ryan Smith | Second paragraph, second sentence: you the get should read you get the |
| 11 | 632 | tech | justin | Handle enumeration explanation fails to address PROCESS_DUP_HANDLE permission. Additional info here. |
| 12 | 736 | typo | Ryan Smith | Fourth paragraph, last sentence: C_IMP_LEVEL_IDENTIFY should read RPC_IMP_LEVEL_IDENTIFY |
| 13 | 805 | typo | tgo | Inline code segment, Line 23: case QUIT should read case QUIT: |
| 17 | 1023 | typo | Rhys | Fourth paragraph, third sentence: arg=h1 should read arg=hi |
| 17 | 1046 | typo | Rhys | Third paragraph, second sentence: "logic and structure are probably be easy to piece together" should read "logic and structure are probably easy to piece together" |
| 17 | 1048 | typo | Rhys | Auditing tip, third sentence: "Determine if any of these unanticipated situations cause a page use" should read "Determine if any of these unanticipated situations cause a page to use" |
Added three errata entries from tgo, but accidentally deleted his comments. Sorry bout that, but thanks for the submissions!
two small typos on page 600. in the first block of code it has “return env->value);” the second block doesnt have a space between int and length.
two small typos on page 805. “if (read_packet_header(sock,&header))
Got them, thanks! Not sure what’s going on with the comments, but I’ll check it out when I get a chance.
Chapter 2, page 27 says ‘monkey market accounts’, which should be ‘money market accounts’. Fun typo.
What appears to be a typo on pg 41 … “Further, asymmetric encryption has no means for verifying the sender of a message among any group of shared key users”.
Shouldn’t that be “symmetric encryption has no means of verifying the sender”, as all users in a group share the same keys? I can see how it could apply to asymmetric encryption and public keys as well, but it would make for an oddly constructed paragraph and not 100% correct.
Typo on pg 1009, end of first paragraph “… which can be retrieved via and HTTP request.”
Obviously ‘and’ is incorrect, but there then is the issue of the oft incorrectly employed indefinite article (a vs. an) before a h-word. Usually ‘an’ is used when the h is silent, or when the acronym is pronounced like a word rather than as individual letters; neither of which apply to ‘HTTP’.
However, Merriam-Webster’s English Usage now recommends that ‘a’ vs. ‘an’ be selected based on which ever pronunciation is easier. Maybe it’s just my Australian accent, but ‘an HTTP request’ sounds better by my reckoning :P
Typo on pg 1023, in relation to GET query strings: The URI is entered into the browser as containing “arg1=h1″, whereas the HTTP request on the network is described as containing “arg1=hi”.
Typo on pg 1046, first dot point of ‘Client Visibility’: “.. the site’s logic and structure are probably be easy to piece together…” extraneous ‘be’ there.
Typo on pg 1048, in the Auditing Tip box: “Determine if any of these unanticipated situations cause a page to use the input without first validating it.” the addition of ‘to’ corrects the sentence.
Much thanks, Rhys. :> I agree with you on the ‘an HTTP request’ thing, so we’ll see what AW says. Again, thanks for all the errata.
I’m hoping to win the award for “most frivolous error,” here’s my first submission:
On page 287, it says “When you read code written by experienced developers, you often see complex expressions that seem to be precariously void of parenthesis.” This should be “overconfident developers.” Experienced developers put in the parenthesis.
lol! That’s a good point. :> I think I wrote that sentence after being frustrated when this one developer kept getting lucky playing fast and loose with precedence, and I eventually decided he was probably just smarter than me.
On page 188:
movl $address_of_argv, %ecx
movb $0xob but no register!
According to the example on page 189, this should be movb $0×0b, %al
Note to self: page 986 - gethostbyname() is not a tough function :>
on page 352 (listing 7-30, page 2)
in read_line():
.
n = read(sockfd, data, sizeof(data)-1);
if (n <= 0)
return -1;
if ((ptr = strchr(data, \n))){
n = ptr - data;
nl = 1;
}
data[n] = ;
.
looks like read_line() can actually corrupt memory on the stack. since the data is read from the wire it doesnt have to be 0-terminated, and using the strchr() function on it is not safe. There could be a chance that it scans beyond the buffer where you read from and for example the saved ebp has a \n in it (or saved eip, whatever, just something thats interesting), n will get adjusted to point to that place, and itll get overwritten with the 0-byte. yea, I know, the stars have to align and all that. anyways, Im assuming this is an unforseen bug in the sample code, since the explanation after listing 7-30 doesnt mention it at all. I guess 0-terminating the string before the use of strchr() would solve it .
Page 87, first paragraph
However, you should aware, Im guessing that should be However, you should be aware.
On page 177 in the first paragraph,
(Remember that authenticated() is at the top of the stack frame.) I think the parenthesis should be removed from around the variable named authenticated, as it makes it look like a function (but its a local variable).
Maybe I am stupid, but I think Listing 6-33 on page 286 is incorrect or totally confusing. The point is structure padding and logic errors leading to a double free. I think the example has an improper use of sizeof() that makes the code broken for a whole different reason. Here goes my analysis:
In free_sechdrs(struct sh *a, struct sh *b) the conditional check is if (!memcmp(a, b, sizeof(a))). a is a pointer to a struct sh object, so sizeof(a) is 4 (or whatever the size of a pointer is on the architecture). As struct sh is defined, this conditional is actually checking whether (a->base == b->base), but the example is really meant to show that memcmp(a, b, sizeof(struct sh)) does not necessarily return the correct logical result even if the structure elements of a and b are equal, given that the padding bits could differ.
Fix: change conditional in free_sechdrs() to if (!memcmp(a, b, sizeof(struct sh)))
page 399
If the username parameter to this function is longer than 1024 bytes, the strlcat() size parameter underflows
but the sample code uses strncat() not strlcat().
chapter 11, missing part of the title.
chapter 9, 10 and 12 start of with: Chapter 9 UNIX I: , Chapter 10 UNIX II: , Chapter 12 WINDOWS II: but for chapter 11 it says Chapter 11 Objects and the File System instead of Chapter 11: WINDOWS I: Object and the File System. this bit me in the ass when I needed to look something up and was looking for WINDOWS I: (all pages in chapter 11 have this)
Hi,
there are two typos in Listing 6-32 on pages 284/285.
The line with the if-statement starts with two opening parentheses even though one parenthesis is enough (the second parenthesis is never closed).
In the same line theres the comparison with g_last_sequence number which should probably be g_last_sequence_number (additional underscore).
Thanks for all the errata everyone!
ilja - good spot on the nul-termination bug. oops! :>
dom - Nice spot and analysis on the structure padding example. It wasnt a particularly great synthetic example to begin with, and that mistake made it completely incorrect and misleading.
Just found a little bug, not security relevant, though. The read_line() function on page 137 may work fine for little endian systems except for the mentioned problem with the 0-byte write but on big endian systems buf is filled with 0s only. Thats because the variable behind the pointer in the read() function is actually an int, not a char. This changes the very first byte of the int in the memory. So for little endian systems thats ok, but for big endian systems this read() changes the most significant byte, not the least significant. So the char value of that int never changes and is 0 all the time.
Futhermore, on page 138 there are some typos regarding the desk-check. The assignments for buf[2] and buf [3] are wrongshould be C and D instead of B.
p. 282: printf(%d\n, i++, i++); should be printf(%d %d\n, i++, i++);
Listing 8-25, IMHO the example code is not correct, even it should show an error code. i think you need to add an dst +=3 after the memmove copy line, or (*dst++ = *src++) will overwrite the first 3 bytes later, results: example code does not work.
Not exactly an errata, but:
On page 489 there is a list of things to look for when permanently dropping privileges. One thing that the book doesnt mention, is that you should make sure that privileged resources are no longer available to the process after the priv drop. Specifically, any file descriptors to privileged files that are no longer needed should be closed.
If there was a file descriptor leak, and the process is compromised via a buffer overflow, or the process execs an attacker-controlled program, then the attacker would have access to a privileged file.
This is mentioned on page 587 in the section on file descriptor leaks. But IMO it should also be in the checklist on page 489.
(From Kurt on the Suggestions page)
Page 208: Ones complement . has the amusing ambiguity of having two values of zero: positive zero and negative zero. This is not amusing, since at the hardware level there is no ambiguity. Since negative zero is less than zero. So, there is a potential security problem, since codes often compare to zero. Since we are talking about integers and not IEEE floats, negative zero and positive zero are the same. ((-x) - (-x)) < (x-x), for positive x, if done directly in hardware as printed, and the hardware does not have extra hardware to check for this special case.
Kurt,
Ah, that sentence is poorly written. The complication for the machine I was referring to was having to implement end-around carry, but Id imagine thats probably not a particularly significant technical barrier. (Im a bit outside of my area of expertise here.) I agree that the ambiguity of negative and positive zero is a human problem of meaning, and not a machine-level issue, as that sentence implies.
Ive never worked on a ones complement machine before, but I absolutely would imagine that it would lead to security exposures.
Its interesting to note that twos complement trades the -0/+0 ambiguity for the one we detail on page 235/236, where for a 32-bit twos complement int, -0×80000000 == 0×80000000.
Congratulations for the excellent book.
Typo: listing 9-4, pg 529: S_IFREG should be S_ISREG
page 426: execve() example at the top should have a NULL terminated array entry for char *args[]
page 380, first sentence following code sample: remove because.
Typo, page 301.
each answer in the DNS response packet (as tracked by anscount), making
According to the provided code (page 300) there is no anscount. It should be ancount.
Typo, page 330.
to the size of the destination buffer buf.
buf on the above sentence is highlighted as a variable but on the given code the destination buffer is named dst. Also, at which writes the NUL one byte past bufs buf should be dst too.
Listing 7-29 (pages 347-348).
if(!(key = read_key(sock))) Should be if(!(k = read_key(sock)))
Typo, page 366.
packet into the ss->sec.ci.ClientChallenge buffer
According to the code in Listing 7-34 it should be ss->sec.ci.clientChallenge
Typo, page 384.
Therefore, whenever new_size() and
This is a variable in Listing 7-47, it should be new_size
Listing 8-2 (page 396) in function is_username_valid() line 6. It
should be strchr(username, :) instead of strchr(name, :)
Page 408.
bob:test
newuseruser:newpassword
Have to be:
bob:test
newuser:newpassword
Page 426.
The command to be executed should be:
/usr/bin/sendmail -s hi user@host.com
However, the 0th element on the args[] is -s
instead of the name of the application.
Page 435, Listing 8-22
if(strchr(bad_characters, *input)
Misssing on more ) at the end.
Page 437.
its the newline ((\n) character.
Should be:
its the newline (\n) character.
Page 452, Listing 8-29
MultiByteToWideChar(CP_ACP, 0, name, strlen(filename), buf, sizeof(buf)/2);
Should probably be:
MultiByteToWideChar(CP_ACP, 0, name, strlen(name), buf, sizeof(buf)/2);
Figure 14-12 on page 873.
The SYN-ACK diagram have 2 Seq fields, the second one were supposed to be Ack: 0xabce.
Page 478, Listing 9-1
[plaguez@plaguez bin]$ ID
On his advisory is:
[plaguez@plaguez bin]$ id
Page 487
The service has an option thatallows
There should be a space:
The service has an option that allows
Page 500
Say a user creates a ~/login.conf file
According to the advisory it should be:
Say a user creates a ~/.login.conf file
Page 518
If homedir is greater than PATH_MAX - strlen(/.optconfig)
there will be truncation.
Page 519
The environment variable SOMEVARcontains
Should probably have a space:
The environment variable SOMEVAR contains
Page 530, Figure 9-8
At the time of check the figure has access() but
the program at Listing 9-4 uses lstat() instead.
Page 556
if(fprintf(fp, %s:%s:%lu:%lu:%s:%s%s\n,
I think it should be:
if(fprintf(fp, %s:%s:%lu:%lu:%s:%s:%s\n,
In order to separate with colon the users
home directory from the users default shell.
Page 616
strncpy(sun.sun_path, path, sizeof(sun.sun_path)-1;
Missing a close parenthesis.
Pages 623-624
At:
switch(rqstp->rq_cred.oa_flavor){
and:
aup = (struct authunix_params *)rqstp->rq_cred;
There is no rqstp svc_req structure, either rename the:
int authenticate(struct svc_req *svc)
on page 623 to rqstp or the above ones to svc.
Thanks for all your hard work tping.. Ill get around to adding these to the list shortly. Very much appreciated.
First of all, thank you too for writing this marvelous book
and sharing this knowledge. Now, here is what Ive found
on chapter 11:
Page 634
the bInheritable parameter 8h a CreateProcess() call
According to MSDN this is named bInheritHandles. The same typo
is also located at the same page, after a few lines:
by setting a true value in the bInheritable member
Here according to MSDN for SECURITY_ATTRIBUTES structure this
should be: bInheritHandle. Lastly, at the end of the first
paragraph:
DuplicateHandle() and passing a true value for the bInheritable
Which once more, according to MSDN should be bInheritHandle.
Page 636
look for the bInheritable parameter in calls to the CreateProcess()
Same as above, bInheritable should be bInheritHandles.
Page 643
HANDLE NewTokenHandle) according to the prototype of
the MSDN: PHANDLE NewTokenHandle.
Page 664
char *ProfileDirectory = c:\profiles;
Should be:
char *ProfileDirectory = c:\\profiles;
Page 671
_snprintf(buf, sizeof(buf), %s\\%s.txt, ProfileDirectory, Name);
Should be:
_snprintf(buf, sizeof(buf), %s\\%s.txt, ProfileDirectory, UserName);
Page 678
snprintf(path, sizeof(path)-1, c:\\temp\\%s_%s_%s.txt, user, filename, ext);
Should be:
snprintf(path, sizeof(path)-1, c:\\temp\\%s_%s_%s.txt, username, filename, ext);
Page 693
ZeroMemory(&wc, sizeof(wnd));
There is no wnd. The block size
should be either sizeof(wc) or sizeof(WNDCLASSEX)
Also the:
return RegisterClassEx( &wnd );
Should be replaced with &wc.
Later, at the next function:
WINDOW hwnd;
Should be:
HWND hwnd; since CreateWindow() returns
this type of object and this variable is used to hold
the return value of CreateWindow().
Next, an MSG structure is used:
GetMessage( &msg, 0, 0, 0 )
But there is no declaration of this variable
in the given functions.
Page 695
Missing a dash on the provided URL, it is:
http://blackhat.com/presentations/bh-usa-04/bh-us-04-moore/bh-us-04-moore whitepaper.pdf
Instead of:
http://blackhat.com/presentations/bh-usa-04/bh-us-04-moore/bh-us-04-moore-whitepaper.pdf
At the same page:
SetProcessWindowStation(hwinsta);
Should be:
SetProcessWindowStation(hWinsta);
Page 701
Return FALSE; Should be: return FALSE;
Page 702
Return FALSE; Should be: return FALSE;
Page 737
if(FAILED(rc))
Return FALSE;
Again it should be: return FALSE;
Page 757
for(list = global_list; list->next; list = list->next);
list->next = element;
Should be:
for(tmp = global_list; tmp->next; tmp = tmp->next);
tmp->next = element;
Page 761
Int thread1(void)
Should be:
int thread1(void)
Page 769
HANDLE CreateSemaphore(LPSECURITY_ATTRIBUTES lpAttributes,
According to MSDN it should be:
HANDLE CreateSemaphore(LPSECURITY_ATTRIBUTES lpSemaphoreAttributes,
At the same page:
HANDLE OpenSemaphore(DWORD dwDesiredAccess, BOOL bInheritable,
Should be:
HANDLE OpenSemaphore(DWORD dwDesiredAccess, BOOL bInheritHandle,
And finally:
HANDLE CreateWaitableTimer(LPSECURITY_ATTRIBUTES lpAttributes,
Should be:
HANDLE CreateWaitableTimer(LPSECURITY_ATTRIBUTES lpTimerAttributes,
Page 770
HANDLE OpenWaitableTimer(DWORD dwDesiredAccess, BOOL bInheritable,
Should be:
HANDLE OpenWaitableTimer(DWORD dwDesiredAccess, BOOL bInheritHandle,
Page 822
Starzets demonstrated how to leverag this bug
Should be:
Starzets demonstrated how to leverage this bug
Page 841
header_length = ntohs(iph->hl);
Should be:
header_length = ntohs(iph->ihl);
Page 855, Figure 14-2:
The last fragment has incorrectly set its MF flag on.
It should not be set.
Page 856
offset = ntohs(iph->frag_offset) * 8;
end = offset + ntohs(iph->tot_len) - iph->hl << 2;
Should be:
offset = ntohs(iph->frag_off) * 8;
end = offset + ntohs(iph->tot_len) - iph->ihl << 2;
And the same element is used on netinet/ip.h iphdr struct.
to retrieve the MF/DF flags, so:
if(!(iph->flags & IP_MF))
Should be:
if(!(iph->frag_off & IP_MF))
Page 901, Figure 15-4
The second node should be renamed from DNS to FTP.
Page 938
int read_header(int soc, char **buffer)
Should be:
int read_header(int sock, char **buffer)
Page 943
tcp_read_data(s, data, clen);
Should be:
tcp_read_data(sock, data, clen);
Page 968
long doi;
Should probably be: unsigned long doi;
how bout a layout bug ? coz those are really important, right ?
on page 781, it says reader_task() in table 13-1 where the first r has a different font than the rest of the function name.
jeebus, can’t believe I have nothing better to do than report a font issue on a friday night ….
Chapter 6 - Section “Pointer Arithmetic” - “Vulnerabilities”
”
int buf[1024];
int *b=buf;
while (havedata() && b < buf + sizeof(buf))
{
*b++=parseint(getdata());
}
The intent of b < buf + sizeof(buf) is to prevent b from advancing past buf[1023]. However, it actually prevents b from advancing past buf[4092]. " <– shouldn’t it be "buf[4095]" ??
Hi All,
I believe the discussion of ‘Handle Inheritance’ (pp. 633-34) has a small verbage issue.
Page 634: “…Windows does provide an explicit mechanism for passing open object instances to its children, called *handle inheritance*.” I believe the correct term is “Object Handle Inheritance”. Confer, Windows via C/C++, p. 43.
Jeff
Hi All,
believe the discussion of ‘Handle Inheritance’ (pp. 633-34) has a small usage issue.
Page 634: “Alternately, the handle can be marked inheritable by calling DuplicateHandle() and passing a TRUE value for for the bInheritable argument.” I believe the most common way to enable inheritance is to use SetHandleInformation() with HANDLE_FLAG_INHERIT. Calling DuplicateHandle() is not necessary and may have the side effect of causing handle table bloat. It’s kind of like killing a fly with a cannon ball. Confer, Windows via C/C++, p. 46.
Jeff
Chapter 2, p.27 Errata: “…monkey market accounts should read money market accounts”
Considering what the Wall Street MBA’s have done, monkey market accounts is probably more appropriate.
Jeff