From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22a.google.com (mail-oi1-x22a.google.com [IPv6:2607:f8b0:4864:20::22a]) by sourceware.org (Postfix) with ESMTPS id 91DF63858C2C for ; Fri, 25 Aug 2023 06:33:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 91DF63858C2C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-oi1-x22a.google.com with SMTP id 5614622812f47-3a88c422e23so401087b6e.0 for ; Thu, 24 Aug 2023 23:33:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692945217; x=1693550017; h=content-disposition:mime-version:message-id:subject:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=kVWF/NCp5ogUvayIQecdXWkNJEYt+09pU4VAI2RRFJg=; b=SDNaVo/Nbgrm38f2DTTGmjn9gC351+U1aWH5tmnd+bdc/3LhMMxMO4Sg8oKBpjW/DV 0LNDn2qWduzlL+PDh0Mfe0OCo1iyA/mSoUF22VRmlVMzkzV0lYQezQjcorzNqqwz7awz /yzEGBsu3uiz735Rkc3RBtblhknM1t+e2DkLLMaVKElz0LlE2l2cJ924r4/cQCPwOsPA ByHm8/HMNqjHUkez7qYu7/nKe3JZN6N/lqRkOM9CpWsjJI3x/DFicrQEcqNVH3kREmPG SAkODO9km/acDgFbUmfd4f6P2DxGyXYFClRGA0QamMyju8oO0ZPzg86eMKZHLqpgUT0j xR6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692945217; x=1693550017; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=kVWF/NCp5ogUvayIQecdXWkNJEYt+09pU4VAI2RRFJg=; b=R4bWt3TrLBlMBy6EKlQroiSiQxRyEC1SARMaPn9pACftVsLojpa4Lof3VAWN68DRIT tiKIOvQn3ujdM4RDMRy3G4yo4OOzxYqP0/7VanXlFpSGrc3+kgVyqDVt3x2XMHf2j9Im ehpBCIsL6WuFQs7vJEN4gREdahYKuBXy91sGNJ3fCVHMb/C6aFxnE2FbTXRta1WE3Zns mcPUuPDx/VMQ8z07EHttEiXxT04SIEShPCy3/fPwz5c1vdFDBLMZFmtg8a6vBVIgeQ6U kXBWzcHjHGhHIhpF5Cwh5InQ29B27fX6gaiBprvShpIaeKNOttaBLf44tAUGZxhkBwdK /k+g== X-Gm-Message-State: AOJu0Yx68zsgaHgetEvCqhiMq3Orye6RheUrOuIsRboPdcAFfA3FyI3v NrcwNyQYSQIOXzbrvwwVe3iH8j5p3DM= X-Google-Smtp-Source: AGHT+IG1GMcQA57YgTZUhe3ZtX3uCY0C30JYsY0wXANK1FQ5ds40LRM3BYuj1dGguqUsELHUr9IFHA== X-Received: by 2002:aca:6502:0:b0:3a3:7612:28cd with SMTP id m2-20020aca6502000000b003a3761228cdmr1774190oim.28.1692945217275; Thu, 24 Aug 2023 23:33:37 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id q10-20020a63bc0a000000b00563ff7d9c4bsm720266pge.73.2023.08.24.23.33.36 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Aug 2023 23:33:36 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D4FE311423DC; Fri, 25 Aug 2023 16:03:32 +0930 (ACST) Date: Fri, 25 Aug 2023 16:03:32 +0930 From: Alan Modra To: binutils@sourceware.org Subject: som: buffer overflow writing strings Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3034.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Code in som_write_symbol_strings neglected to allow for padding, which can result in a buffer overflow. It also used xrealloc, which we're not supposed to use in libbfd because libbfd isn't supposed to call exit. Also a realloc is perhaps not a good idea when none of the buffer contents are needed, so replace with free, bfd_malloc. There were three copies of the string handling code, so rather than fix them all I've extracted them to a function. This necessitated making one of the fields in struct som_symbol unsigned. * som.c (add_string): New function. (som_write_space_strings, som_write_symbol_strings): Use it. * som.h (som_symbol_type ): Make unsigned. diff --git a/bfd/som.c b/bfd/som.c index ac2c4e44864..d858b8b1468 100644 --- a/bfd/som.c +++ b/bfd/som.c @@ -3304,22 +3304,89 @@ som_write_fixups (bfd *abfd, return true; } +/* Write the length of STR followed by STR to P which points into + *BUF, a buffer of *BUFLEN size. Track total size in *STRINGS_SIZE, + setting *STRX to the current offset for STR. When STR can't fit in + *BUF, flush the buffer to ABFD, possibly reallocating. Return the + next available location in *BUF, or NULL on error. */ + +static char * +add_string (char *p, const char *str, bfd *abfd, char **buf, size_t *buflen, + unsigned int *strings_size, unsigned int *strx) +{ + size_t length = strlen (str) + 1; + /* Each entry will take 4 bytes to hold the string length + the + string itself + null terminator + padding to a 4 byte boundary. */ + size_t needed = (4 + length + 3) & ~3; + + /* If there is not enough room for the next entry, then dump the + current buffer contents now and maybe allocate a larger buffer. */ + if (p - *buf + needed > *buflen) + { + /* Flush buffer before refilling or reallocating. */ + size_t amt = p - *buf; + if (bfd_write (*buf, amt, abfd) != amt) + return NULL; + + /* Reallocate if now empty buffer still too small. */ + if (needed > *buflen) + { + /* Ensure a minimum growth factor to avoid O(n**2) space + consumption for n strings. The optimal minimum factor + seems to be 2. */ + if (*buflen * 2 < needed) + *buflen = needed; + else + *buflen = *buflen * 2; + free (*buf); + *buf = bfd_malloc (*buflen); + if (*buf == NULL) + return NULL; + } + + /* Reset to beginning of the (possibly new) buffer space. */ + p = *buf; + } + + /* First element in a string table entry is the length of + the string. This must always be 4 byte aligned. This is + also an appropriate time to fill in the string index + field in the symbol table entry. */ + bfd_put_32 (abfd, length - 1, p); + *strings_size += 4; + p += 4; + + *strx = *strings_size; + + /* Next comes the string itself + a null terminator. */ + memcpy (p, str, length); + p += length; + *strings_size += length; + + /* Always align up to the next word boundary. */ + if (length & 3) + { + length = 4 - (length & 3); + memset (p, 0, length); + *strings_size += length; + p += length; + } + return p; +} + /* Write out the space/subspace string table. */ static bool som_write_space_strings (bfd *abfd, unsigned long current_offset, - unsigned int *string_sizep) + unsigned int *strings_size) { /* Chunk of memory that we can use as buffer space, then throw away. */ size_t tmp_space_size = SOM_TMP_BUFSIZE; char *tmp_space = bfd_malloc (tmp_space_size); char *p = tmp_space; - unsigned int strings_size = 0; asection *section; - size_t amt; - bfd_size_type res; if (tmp_space == NULL) return false; @@ -3331,86 +3398,32 @@ som_write_space_strings (bfd *abfd, /* Walk through all the spaces and subspaces (order is not important) building up and writing string table entries for their names. */ + *strings_size = 0; for (section = abfd->sections; section != NULL; section = section->next) { - size_t length; + unsigned int *strx; /* Only work with space/subspaces; avoid any other sections which might have been made (.text for example). */ - if (!som_is_space (section) && !som_is_subspace (section)) - continue; - - /* Get the length of the space/subspace name. */ - length = strlen (section->name); - - /* If there is not enough room for the next entry, then dump the - current buffer contents now and maybe allocate a larger - buffer. Each entry will take 4 bytes to hold the string - length + the string itself + null terminator. */ - if (p - tmp_space + 5 + length > tmp_space_size) - { - /* Flush buffer before refilling or reallocating. */ - amt = p - tmp_space; - if (bfd_write (&tmp_space[0], amt, abfd) != amt) - return false; - - /* Reallocate if now empty buffer still too small. */ - if (5 + length > tmp_space_size) - { - /* Ensure a minimum growth factor to avoid O(n**2) space - consumption for n strings. The optimal minimum - factor seems to be 2, as no other value can guarantee - wasting less than 50% space. (Note that we cannot - deallocate space allocated by `alloca' without - returning from this function.) The same technique is - used a few more times below when a buffer is - reallocated. */ - if (2 * tmp_space_size < length + 5) - tmp_space_size = length + 5; - else - tmp_space_size = 2 * tmp_space_size; - tmp_space = xrealloc (tmp_space, tmp_space_size); - } - - /* Reset to beginning of the (possibly new) buffer space. */ - p = tmp_space; - } - - /* First element in a string table entry is the length of the - string. Alignment issues are already handled. */ - bfd_put_32 (abfd, (bfd_vma) length, p); - p += 4; - strings_size += 4; - - /* Record the index in the space/subspace records. */ if (som_is_space (section)) - som_section_data (section)->space_dict->name = strings_size; + strx = &som_section_data (section)->space_dict->name; + else if (som_is_subspace (section)) + strx = &som_section_data (section)->subspace_dict->name; else - som_section_data (section)->subspace_dict->name = strings_size; - - /* Next comes the string itself + a null terminator. */ - strcpy (p, section->name); - p += length + 1; - strings_size += length + 1; + continue; - /* Always align up to the next word boundary. */ - while (strings_size % 4) - { - bfd_put_8 (abfd, 0, p); - p++; - strings_size++; - } + p = add_string (p, section->name, abfd, &tmp_space, &tmp_space_size, + strings_size, strx); + if (p == NULL) + return false; } /* Done with the space/subspace strings. Write out any information contained in a partial block. */ - amt = p - tmp_space; - res = bfd_write (&tmp_space[0], amt, abfd); + size_t amt = p - tmp_space; + bool ok = amt ? bfd_write (tmp_space, amt, abfd) == amt : true; free (tmp_space); - if (res != amt) - return false; - *string_sizep = strings_size; - return true; + return ok; } /* Write out the symbol string table. */ @@ -3420,7 +3433,7 @@ som_write_symbol_strings (bfd *abfd, unsigned long current_offset, asymbol **syms, unsigned int num_syms, - unsigned int *string_sizep, + unsigned int *strings_size, struct som_compilation_unit *compilation_unit) { unsigned int i; @@ -3429,9 +3442,6 @@ som_write_symbol_strings (bfd *abfd, size_t tmp_space_size = SOM_TMP_BUFSIZE; char *tmp_space = bfd_malloc (tmp_space_size); char *p = tmp_space; - unsigned int strings_size = 0; - size_t amt; - bfd_size_type res; if (tmp_space == NULL) return false; @@ -3448,12 +3458,12 @@ som_write_symbol_strings (bfd *abfd, if (bfd_seek (abfd, current_offset, SEEK_SET) != 0) return false; + *strings_size = 0; if (compilation_unit) { for (i = 0; i < 4; i++) { struct som_name_pt *name; - size_t length; switch (i) { @@ -3473,121 +3483,28 @@ som_write_symbol_strings (bfd *abfd, abort (); } - length = strlen (name->name); - - /* If there is not enough room for the next entry, then dump - the current buffer contents now and maybe allocate a - larger buffer. */ - if (p - tmp_space + 5 + length > tmp_space_size) - { - /* Flush buffer before refilling or reallocating. */ - amt = p - tmp_space; - if (bfd_write (tmp_space, amt, abfd) != amt) - return false; - - /* Reallocate if now empty buffer still too small. */ - if (5 + length > tmp_space_size) - { - /* See alloca above for discussion of new size. */ - if (2 * tmp_space_size < 5 + length) - tmp_space_size = 5 + length; - else - tmp_space_size = 2 * tmp_space_size; - tmp_space = xrealloc (tmp_space, tmp_space_size); - } - - /* Reset to beginning of the (possibly new) buffer - space. */ - p = tmp_space; - } - - /* First element in a string table entry is the length of - the string. This must always be 4 byte aligned. This is - also an appropriate time to fill in the string index - field in the symbol table entry. */ - bfd_put_32 (abfd, (bfd_vma) length, p); - strings_size += 4; - p += 4; - - /* Next comes the string itself + a null terminator. */ - strcpy (p, name->name); + p = add_string (p, name->name, abfd, &tmp_space, &tmp_space_size, + strings_size, &name->strx); - name->strx = strings_size; - - p += length + 1; - strings_size += length + 1; - - /* Always align up to the next word boundary. */ - while (strings_size % 4) - { - bfd_put_8 (abfd, 0, p); - strings_size++; - p++; - } + if (p == NULL) + return false; } } for (i = 0; i < num_syms; i++) { - size_t length = strlen (syms[i]->name); - - /* If there is not enough room for the next entry, then dump the - current buffer contents now and maybe allocate a larger buffer. */ - if (p - tmp_space + 5 + length > tmp_space_size) - { - /* Flush buffer before refilling or reallocating. */ - amt = p - tmp_space; - if (bfd_write (tmp_space, amt, abfd) != amt) - return false; - - /* Reallocate if now empty buffer still too small. */ - if (5 + length > tmp_space_size) - { - /* See alloca above for discussion of new size. */ - if (2 * tmp_space_size < 5 + length) - tmp_space_size = 5 + length; - else - tmp_space_size = 2 * tmp_space_size; - tmp_space = xrealloc (tmp_space, tmp_space_size); - } - - /* Reset to beginning of the (possibly new) buffer space. */ - p = tmp_space; - } - - /* First element in a string table entry is the length of the - string. This must always be 4 byte aligned. This is also - an appropriate time to fill in the string index field in the - symbol table entry. */ - bfd_put_32 (abfd, (bfd_vma) length, p); - strings_size += 4; - p += 4; - - /* Next comes the string itself + a null terminator. */ - strcpy (p, syms[i]->name); - - som_symbol_data (syms[i])->stringtab_offset = strings_size; - p += length + 1; - strings_size += length + 1; - - /* Always align up to the next word boundary. */ - while (strings_size % 4) - { - bfd_put_8 (abfd, 0, p); - strings_size++; - p++; - } + p = add_string (p, syms[i]->name, abfd, &tmp_space, &tmp_space_size, + strings_size, + &som_symbol_data (syms[i])->stringtab_offset); + if (p == NULL) + return false; } /* Scribble out any partial block. */ - amt = p - tmp_space; - res = bfd_write (tmp_space, amt, abfd); + size_t amt = p - tmp_space; + bool ok = amt ? bfd_write (tmp_space, amt, abfd) == amt : true; free (tmp_space); - if (res != amt) - return false; - - *string_sizep = strings_size; - return true; + return ok; } /* Compute variable information to be placed in the SOM headers, diff --git a/bfd/som.h b/bfd/som.h index 8152cfc27d3..a6f91a0082b 100644 --- a/bfd/som.h +++ b/bfd/som.h @@ -81,7 +81,7 @@ typedef struct som_symbol /* During object file writing, the offset of the name of this symbol in the SOM string table. */ - int stringtab_offset; + unsigned int stringtab_offset; } som_symbol_type; -- Alan Modra Australia Development Lab, IBM