html/template,text/template: use errors.New instead of fmt.Errorf

[austinderek]

For non-formatted strings "fmt.Errorf" and "errors.New" have the same result,
but the former is slower and allocates more memory.

So it is better to call "errors.New" directly for non-formatted strings.

---

🔄 This is a mirror of upstream PR golang#75719

[staging]

HackerOne Code Security Review

🟢 Scan Complete: 44 Issue(s)
🟠 Validation Complete: One or more Issues looked potentially actionable, so this was escalated to our network of engineers for manual review. Once this is complete you'll see an update posted.

Here's how the code changes were interpreted and info about the tools used for scanning.

📖 Summary of Changes

The changes across multiple template-related Go files involve systematically replacing `fmt.Errorf()` with `errors.New()` for creating simple, static error messages. This modification simplifies error generation in template parsing and function handling, primarily in the `src/html/template` and `src/text/template` packages, with a focus on more direct error message creation.

File	Summary
src/html/template/template.go	The changes include replacing some fmt.Errorf() calls with errors.New(), specifically in the checkCanParse() method and parseFiles() function. The code now uses the errors package for creating simple error messages instead of formatting them.
src/text/template/funcs.go	The changes involve replacing some direct error creation with errors.New() instead of fmt.Errorf() in several error-generating functions, such as indexArg(), index(), slice(), length(), and call(). This simplifies error generation for specific, static error messages.
src/text/template/helper.go	The primary change is replacing fmt.Errorf("template: no files named in call to ParseFiles") with errors.New("template: no files named in call to ParseFiles"), and adding an errors import to the import block.

ℹ️ Issues Detected

NOTE: These may not require action!

Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.

How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:

• Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
• Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.

File & Line	Issue
src/cmd/compile/internal/ssa/rewriteWasm.go Line 3204	The code has removed a dependency on internal/buildcfg and also removed conditional code paths that were using buildcfg.GOWASM.SignExt. The sign extension operations now always use the direct WebAssembly instructions (I64Extend8S, I64Extend16S, I64Extend32S) without checking if they're supported by the target. This could potentially cause compatibility issues if these instructions aren't available in all WebAssembly environments the code is targeting.
src/mime/mediatype.go Line 345	The percentHexUnescape function has been modified to return a boolean instead of an error. This change could lead to silent failures where previously errors would be reported. The function now returns (string, bool) instead of (string, error), and error cases that previously returned detailed error messages now just return false without any context about what went wrong.
src/debug/elf/file.go Line 1834	The new putUint function has a potential integer overflow vulnerability. In line 1834, the condition math.MaxUint64-start < length is intended to prevent integer overflow when checking bounds, but it doesn't fully protect against all overflow cases. The function also doesn't check for overflow when adding sym+ae in lines 1849 and 1855, which could lead to incorrect values being written if the sum exceeds the maximum value for the respective data type.
src/internal/poll/fd_windows.go Line 266	The code introduces runtime.Pinner to pin memory during I/O operations, but there's a potential memory safety issue in the pin/unpin pattern. In the execIO function, the operation object 'o' is pinned but then put back into the operationPool before the deferred unpinning happens. This could lead to a situation where memory is unpinned after it's already been reused by another goroutine, causing memory corruption.
src/cmd/link/internal/ld/macho.go Line 109	These changes add two new constants for Mach-O ARM64 relocation types. The additions are just constant definitions and don't introduce any security vulnerabilities. The constants MACHO_ARM64_RELOC_SUBTRACTOR and MACHO_ARM64_RELOC_POINTER_TO_GOT are standard relocation types used in the Mach-O binary format for ARM64 architecture.
src/runtime/conv_wasm_test.go Line 26	The changes to the test expectations for float-to-integer conversions indicate that the behavior of these conversions has changed. If this is due to a change in the runtime implementation (removal of wasmTruncS/wasmTruncU functions in sys_wasm.s), it could affect the behavior of applications that rely on specific overflow/underflow handling for float-to-integer conversions in WebAssembly.
src/cmd/link/internal/ld/symtab.go Line 648	The change on line 648 modifies how the cutab slice is handled in the moduledata. The original code used sliceSym which would set the length and capacity to the full size of the symbol. The new code explicitly divides the size by 4, which suggests the cutab contains 4-byte elements. If the cutab's size is not actually a multiple of 4, this could lead to incorrect memory access or buffer overflows when the runtime uses this data.
src/testing/synctest/synctest.go Line 150	The error message in the test example was corrected from 'before context is canceled: AfterFunc not called' to 'after context is canceled: AfterFunc not called'. This is a documentation fix that improves accuracy but doesn't introduce security vulnerabilities.
src/runtime/asm_ppc64x.s Line 39	The added code introduces a new function _rt0_ppc64x_lib that handles library initialization. The function uses a hardcoded stack size of 8192KB (line 39) without validation. While this is common in low-level runtime code, it's worth noting that this fixed allocation might be excessive for some environments or insufficient for others. Consider whether this size should be configurable or determined dynamically based on the environment.
src/cmd/cgo/internal/testcshared/cshared_test.go Line 276	The change removes _cgo_dummy_export from the Windows DLL export list. This is a security improvement as it reduces the attack surface by removing an unnecessary exported symbol. The code now correctly handles the case where there are no exported symbols by using cmp.Or(exportedSymbols, 1) to ensure at least one symbol is expected.
src/cmd/link/internal/loader/loader.go Line 1115	The new ForAllCgoExportStatic function uses the iter package to create an iterator over symbols marked with the "cgo_export_static" compiler directive. This introduces a potential security issue if the caller doesn't properly handle early termination of the iteration. If the yield function returns false to stop iteration, but the caller doesn't check for this condition, it could lead to incomplete processing of security-critical symbols. This is especially concerning for CGO exports which often involve interactions between Go and C code, a common source of security vulnerabilities.
src/crypto/internal/fips140/mlkem/mlkem768.go Line 175	The added fipsSelfTest() function calls on lines 175, 189, 342, 354, and 460 are being invoked without checking their return values. If this function is meant to perform security validation and can fail, not handling potential errors could lead to the system operating in an insecure state. The code should either check the return value or ensure the function panics on failure.
src/runtime/export_test.go Line 1940	The added lines expose a debug flag DebugDecorateMappings and a function SetVMANameSupported() to the test package. While these additions themselves don't introduce security vulnerabilities, exposing internal debug flags to test code could potentially be misused if the test package is imported by non-test code in production. However, this is a standard pattern in Go for testing and the risk is minimal since these are only exported for testing purposes.
src/crypto/internal/fips140/mlkem/cast.go Line 15	The change introduces a potential race condition. The fipsSelfTest function is defined but never called in the init function. This means the CAST self-test for ML-KEM-768 might never run, which could lead to using cryptographic functionality without proper validation in FIPS mode.
src/encoding/xml/read.go Line 261	The code changes replace direct type assertions with reflect.TypeAssert, but the implementation is vulnerable to type confusion. The new code checks if val.CanInterface() but doesn't verify the type before attempting the type assertion with reflect.TypeAssert. If the value doesn't implement the expected interface, the code will panic at runtime rather than gracefully handling the error.
src/cmd/link/internal/ld/symtab.go Line 657	On line 657, the code changes from using a custom slice function with explicit size to using sliceSym. This change appears to be safer as it uses the actual symbol size rather than a potentially incorrect manual calculation, reducing the risk of buffer overflows.
src/cmd/cgo/out.go Line 1009	The code removes the __declspec(dllexport) attribute from exported functions in Windows builds. This could prevent proper symbol exporting when building DLLs on Windows, potentially making exported functions inaccessible to external code that tries to dynamically link against the library.
src/cmd/link/internal/ld/pe.go Line 1788	The function peCreateExportFile writes a .def file to a location specified by the *flagTmpdir flag without proper path validation or sanitization. This could potentially lead to path traversal issues if the flagTmpdir value contains malicious path components. Additionally, the file is created with 0666 permissions, making it world-readable and writable, which could be a security concern in shared environments.
src/cmd/compile/internal/ssa/_gen/Wasm.rules Line 58	The changes simplify the sign extension operations by removing conditional checks for the buildcfg.GOWASM.SignExt feature flag. The code now directly uses WebAssembly's native sign extension instructions (I64Extend32S, I64Extend8S, and I64Extend16S) instead of conditionally using them or falling back to a more complex implementation using shifts. This change doesn't introduce security vulnerabilities but does change the compiler's behavior to always use these native instructions.
src/cmd/link/dwarf_test.go Line 367	The change adds a check for DWARF support on the current platform before running the TestFlagW test. This is a positive security change that prevents the test from running on unsupported platforms, avoiding potential test failures or undefined behavior. No security vulnerabilities are introduced by this change.
src/crypto/internal/fips140/mlkem/mlkem1024.go Line 116	The added fipsSelfTest() calls in multiple functions (generateKey1024, GenerateKeyInternal1024, encapsulate, EncapsulateInternal, Decapsulate) could potentially introduce a security issue if the self-test fails silently. The code doesn't show how errors from fipsSelfTest() are handled, which might allow cryptographic operations to proceed even when the self-test fails. This could lead to using a cryptographic implementation that doesn't meet FIPS requirements.
src/crypto/internal/fips140test/entropy_test.go Line 40	The test file contains a potential security issue in the TestEntropySamples function. When flagEntropySamples is set, it writes entropy samples to files with 0644 permissions. Writing entropy data to disk with world-readable permissions could expose sensitive randomness patterns to other users on the system, potentially weakening cryptographic operations if an attacker can analyze these patterns.
src/cmd/compile/internal/ssa/rewrite.go Line 2062	The code changes modify how type information is accessed from symbols. Instead of directly accessing lsym.Extra and checking if it's a *obj.TypeInfo or *obj.ItabInfo, the code now uses helper methods lsym.TypeInfo() and lsym.ItabInfo(). This is a safer approach as it encapsulates the type checking and casting logic, but doesn't introduce any security vulnerabilities.
src/cmd/link/internal/ld/xcoff.go Line 1782	The change replaces a manual iteration over all symbols to check for AttrCgoExport with a call to ldr.ForAllCgoExportStatic(). This is a safer approach as it uses a dedicated method to iterate over the specific symbols needed, rather than manually filtering. However, the new code doesn't check if the returned symbol is valid before using it, which could potentially lead to issues if the method returns invalid symbols.
src/runtime/runtime1.go Line 405	The function parsedebugvars() has been replaced with parseRuntimeDebugVars(godebug string), which now takes a string parameter instead of reading from the environment directly. This change removes the direct environment variable access from this function, which is a security improvement as it allows for more controlled access to environment variables. The caller can now validate or sanitize the input before passing it to this function.

🧰 Analysis tools

• [ ✅ ] HackerOne AI Code Analysis
• [ ✅ ] HackerOne AI Code Validation
• [ ✅ ] semgrep
• [ ✅ ] gosec
• [ ✅ ] bandit

⏱️ Latest scan covered changes up to commit a0d7e69 (latest)