From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2a.google.com (mail-io1-xd2a.google.com [IPv6:2607:f8b0:4864:20::d2a]) by sourceware.org (Postfix) with ESMTPS id 3C1353952010 for ; Thu, 17 Mar 2022 10:58:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3C1353952010 Received: by mail-io1-xd2a.google.com with SMTP id 195so5471440iou.0 for ; Thu, 17 Mar 2022 03:58:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition; bh=7Ko9cIAywCfsU7CCbcS6vZXy67J8IuOVP+SA0XwhOQU=; b=uRnJYDWNx3N2i2NkzjFe6QrXpd5cykQNTg3lopp6HA9/fxclLTFz4kTueT5lBr7/TN 8HWhF7OAIWGNzgL7ZSmK6JGrFHU6bghHFKODfgQN9XrIZ8tZJDTvaDLa6q4oRyerkEHC Rs63f7Ut3FfmRzDAcnH+DwrC1litjg6T654hm/MJAfFVzYqzoW9ya+0PnZnrcaABzGat 7WLZzONCSvsChMiTGUR/mZKHTYRpO3Nc3AomOH6CNY0czP8BULBxDlpxE62h1L0Gw5Zu ebjhHHgbiT0zlyXB8xj9FhY/VKtlRoa+vn9U6+Jwpn1ZQtfQcn2Km2qz4cGYTUSJuXsx 2qiQ== X-Gm-Message-State: AOAM531JwdzubhAlFx7daCnD0OI0bnVGPE3qfkO2jO95nK++HXk8s4rO v7k+zLNjbx1ydEoZQgqeePWXzwgNw4Y= X-Google-Smtp-Source: ABdhPJzhcNOqmQktoGqKG2nVUR1CZqYv9NmAhbEO6nqscYnNLsNp+nGHlSx3Nb0Z9X8N/43te7bSgw== X-Received: by 2002:a05:6638:1655:b0:319:a174:6ba0 with SMTP id a21-20020a056638165500b00319a1746ba0mr1792253jat.195.1647514735133; Thu, 17 Mar 2022 03:58:55 -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 b6-20020a056602000600b006494b91cecdsm998774ioa.0.2022.03.17.03.58.54 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 17 Mar 2022 03:58:54 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D68D91142F41; Thu, 17 Mar 2022 21:28:51 +1030 (ACDT) Date: Thu, 17 Mar 2022 21:28:51 +1030 From: Alan Modra To: binutils@sourceware.org Subject: Re: asan: buffer overflow in peXXigen.c Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3037.7 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, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Mar 2022 10:58:57 -0000 In the process of fixing a buffer overflow in commit fe69d4fcf0194a, I managed to introduce a fairly obvious NULL pointer dereference.. * peXXigen.c (_bfd_XX_bfd_copy_private_bfd_data_common): Don't segfault on not finding section. Wrap overlong lines. diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index d11ea01c554..50e4face50c 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -2984,64 +2984,72 @@ _bfd_XX_bfd_copy_private_bfd_data_common (bfd * ibfd, bfd * obfd) one. */ bfd_vma last = addr + size - 1; asection *section = find_section_by_vma (obfd, last); - bfd_byte *data; - bfd_vma dataoff = addr - section->vma; - /* PR 17512: file: 0f15796a. */ - if (section - && (addr < section->vma - || section->size < dataoff - || section->size - dataoff < size)) + if (section != NULL) { - /* xgettext:c-format */ - _bfd_error_handler - (_("%pB: Data Directory (%lx bytes at %" PRIx64 ") " - "extends across section boundary at %" PRIx64), - obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size, - (uint64_t) addr, (uint64_t) section->vma); - return false; - } + bfd_byte *data; + bfd_vma dataoff = addr - section->vma; - if (section && bfd_malloc_and_get_section (obfd, section, &data)) - { - unsigned int i; - struct external_IMAGE_DEBUG_DIRECTORY *dd = - (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff); + /* PR 17512: file: 0f15796a. */ + if (addr < section->vma + || section->size < dataoff + || section->size - dataoff < size) + { + /* xgettext:c-format */ + _bfd_error_handler + (_("%pB: Data Directory (%lx bytes at %" PRIx64 ") " + "extends across section boundary at %" PRIx64), + obfd, ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size, + (uint64_t) addr, (uint64_t) section->vma); + return false; + } - for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size - / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++) + if (bfd_malloc_and_get_section (obfd, section, &data)) { - asection *ddsection; - struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]); - struct internal_IMAGE_DEBUG_DIRECTORY idd; + unsigned int i; + struct external_IMAGE_DEBUG_DIRECTORY *dd = + (struct external_IMAGE_DEBUG_DIRECTORY *)(data + dataoff); + + for (i = 0; i < ope->pe_opthdr.DataDirectory[PE_DEBUG_DATA].Size + / sizeof (struct external_IMAGE_DEBUG_DIRECTORY); i++) + { + asection *ddsection; + struct external_IMAGE_DEBUG_DIRECTORY *edd = &(dd[i]); + struct internal_IMAGE_DEBUG_DIRECTORY idd; + bfd_vma idd_vma; - _bfd_XXi_swap_debugdir_in (obfd, edd, &idd); + _bfd_XXi_swap_debugdir_in (obfd, edd, &idd); - if (idd.AddressOfRawData == 0) - continue; /* RVA 0 means only offset is valid, not handled yet. */ + /* RVA 0 means only offset is valid, not handled yet. */ + if (idd.AddressOfRawData == 0) + continue; - ddsection = find_section_by_vma (obfd, idd.AddressOfRawData + ope->pe_opthdr.ImageBase); - if (!ddsection) - continue; /* Not in a section! */ + idd_vma = idd.AddressOfRawData + ope->pe_opthdr.ImageBase; + ddsection = find_section_by_vma (obfd, idd_vma); + if (!ddsection) + continue; /* Not in a section! */ - idd.PointerToRawData = ddsection->filepos + (idd.AddressOfRawData - + ope->pe_opthdr.ImageBase) - ddsection->vma; + idd.PointerToRawData + = ddsection->filepos + idd_vma - ddsection->vma; + _bfd_XXi_swap_debugdir_out (obfd, &idd, edd); + } - _bfd_XXi_swap_debugdir_out (obfd, &idd, edd); + if (!bfd_set_section_contents (obfd, section, data, 0, + section->size)) + { + _bfd_error_handler (_("failed to update file offsets" + " in debug directory")); + free (data); + return false; + } + free (data); } - - if (!bfd_set_section_contents (obfd, section, data, 0, section->size)) + else { - _bfd_error_handler (_("failed to update file offsets in debug directory")); - free (data); + _bfd_error_handler (_("%pB: failed to read " + "debug data section"), obfd); return false; } - free (data); - } - else if (section) - { - _bfd_error_handler (_("%pB: failed to read debug data section"), obfd); - return false; } } -- Alan Modra Australia Development Lab, IBM