Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15205 closed defect (fixed)

There's something fishy in OSX's checked strlcat.

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.5.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: 025-backport, 2016-bug-retrospective
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Have a look at https://opensource.apple.com/source/Libc/Libc-1044.1.2/secure/strlcat_chk.c .

When it checks to see whether the destination buffer overlaps with the input buffer in the second case, it isn't checking whether the input overlaps with the actual buffer that *will* be written; it's checking whether the input overlaps with the destination buffer, plus the extra space after the end of the destination buffer that would be written if there were enough room for the whole input.

I believe the second overlap check should be something more like:

    __chk_overlap(dest, len, src, len - initial_dstlen - 1);

To do:

  • Check whether any of the BSDs have this problem.
  • Check whether older OSXs have this problem.

Child Tickets

Change History (9)

comment:1 Changed 4 years ago by nickm

This appears not to be in any of the BSDs. It appears to be new in OSX 10.9 and later.

Anybody know how to file an OSX Libc bug?

comment:2 Changed 4 years ago by nickm

Here is a C file to reproduce the bug:

#include <string.h>

int main(int argc, char **argv)
{
 char a[300];
 char b[30];
 char c[300];

 memset(a, 'x', 299);
 a[299] = 0;
 memset(c, 'y', 299);
 c[299] = 0;
 
 b[0] = 0;
 strlcat(b, a, 30);
 
 b[0] = 0;
 strlcat(b, c, 30);

 return 0;
}

Compile it with GCC on OSX 10.10, run it, and you'll get "Abort trap: 6". I believe 10.9 will have the same issue.

(Edited to fix a bug in the code; it still fails with an abort trap)

Last edited 4 years ago by nickm (previous) (diff)

comment:3 Changed 4 years ago by nickm

Keywords: 025-backport added

comment:4 Changed 4 years ago by nickm

I have a workaround tested in bug15205_025.

comment:5 Changed 4 years ago by nickm

Status: newneeds_review

comment:6 Changed 4 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.5.x-final

Merged to 0.2.6; if nothing breaks, let's backport to 0.2.5

comment:7 Changed 4 years ago by nickm

(A kind soul has filed a bug report with apple. Thanks, Andreas!)

comment:8 Changed 4 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Backporting to 0.2.5, and no further.

comment:9 Changed 3 years ago by nickm

Keywords: 2016-bug-retrospective added

Mark the last set of tickets that came up in my last-few-years changelog hand-review.

Note: See TracTickets for help on using tickets.