an FPSetACL request can crash afpd if the ace_count is too big

[rtmrtmrtmrtm]

afp_setacl() reads the client's ace_count without checking against
the message size:

    uint32_t ace_count;
    memcpy(&ace_count, ibuf, sizeof(uint32_t));
    ace_count = htonl(ace_count);
    ibuf += 8;      /* skip ACL flags (see acls.h) */

    ret = set_acl(vol,
                  s_path->u_name,
                  (bitmap & kFileSec_Inherit),
                  (darwin_ace_t *)ibuf,
                  ace_count);

set_acl() passes ibuf (as darwin_aces) and ace_count to
map_aces_darwin_to_posix(), which will run off the end of the message
buffer and perhaps crash if ace_count is too big:

for ( ; ace_count != 0; ace_count--, darwin_aces++) {
    darwin_ace_flags = ntohl(darwin_aces->darwin_ace_flags);

Here's a backtrace from a core dump:

#0 0x0000000000253ccb in map_aces_darwin_to_posix
(darwin_aces=0x8423e9ff6, def_aclp=0x820273930, acc_aclp=0x820273928, ace_count=268391768, default_acl_flags=0x820273914) at acls.c:823
#1 0x0000000000251f90 in set_acl
(vol=0x839b3ee20, name=0x25dca0 <mtoupath[upath]> "1", inherit=16, daces=0x8422ea036, ace_count=268435456) at acls.c:1344
#2 0x0000000000251a7a in afp_setacl
(obj=0x2641a0 , ibuf=0x8422ea036 "", ibuflen=112, rbuf=0x8411736f0 "\001", rbuflen=0x8411836f0) at acls.c:1746
#3 0x0000000000219e27 in afp_over_dsi (obj=0x2641a0 ) at afp_dsi.c:627
#4 0x0000000000242dc0 in dsi_start
(obj=0x2641a0 , dsi=0x841173000, server_children=0x839b178c0)
at main.c:474
#5 0x00000000002423f6 in main (ac=2, av=0x820273c88) at main.c:417

[rdmark]

Thanks for reporting the bug!

Do you have concrete reproduction steps?

And if you have a fix in mind please by all means file a PR. We very much welcome code contributions. :)

[rtmrtmrtmrtm]

To reproduce, you could temporarily modify afp_setacl() to set
ace_count to a huge value at the place in the code where it
reads the message, i.e. replace

    memcpy(&ace_count, ibuf, sizeof(uint32_t));
    ace_count = htonl(ace_count);

with

    ace_count = 1000000000;

I don't have much experience with netatalk, and I don't have a test
suite to check that a fix doesn't break anything, but you could
consider modifying the start of afp_setacl() to save ibuf and ibuflen:

size_t ibuflen0 = ibuflen;
char *ibuf0 = ibuf;

and then checking ace_count before the call to set_acl():

    if(ibuf + ace_count*sizeof(darwin_ace_t) > ibuf0 + ibuflen0){
        LOG(log_error, logtype_afpd, "afp_setacl: ace_count is too big");
        return AFPERR_MISC;
    }

[rdmark]

@rtmrtmrtmrtm A PR with your proposal here: #546

To your point, some testing will be required to make sure nothing breaks. I'm currently living out of a hotel with only my MacBook, so my options are limited for thorough testing. We have a test suite in a parallel repo that I recently got running on macOS: https://github.com/Netatalk/afptest