Commit Graph
7780 Commits
Author SHA1 Message Date
Michael Brown 11b793c586 [tls] Treat secure channel's cipher algorithm as definitive
The transmit and receive ciphers are now owned by the secure channel.
We assert at the point of use that the cipher algorithm used by the
secure channel matches the algorithm in the currently active cipher
specification's cipher suite, but this does not guard against future
code changes that could accidentally leave these out of sync, and
thereby end up passing an incorrectly sized context to the cipher.

Fix by treating the secure channel as having the definitive record of
the current cipher algorithm, and treating the algorithm specified in
the cipher suite as being used only as a parameter for initialising
the transmit or receive pipe via the secure channel.

This matches the way that we treat the key schedule as having the
definitive record of the current handshake digest algorithm, with the
algorithm specified in the cipher suite being used only as a parameter
for initialising the key schedule.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 21:17:01 +01:00
Michael Brown dc962df26f [tls] Ensure that ServerKeyExchange parsing method is always non-NULL
Use a dedicated parser function for unexpected ServerKeyExchange
records, to create an invariant that the parser function field always
has a non-NULL value.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 20:00:00 +01:00
Michael Brown c6e8c42238 [tls] Use null key exchange algorithm as default for DHE and ECDHE
The DHE and ECDHE ServerKeyExchange parsers will always return the
identified key exchange algorithm in the parsed parameters.  The
default key exchange algorithm is therefore never used for these key
exchange mechanisms.

Set the default key exchange algorithm for DHE and ECDHE to be the
null key exchange algorithm (which will always fail operations), and
also set this as the in-use key exchange algorithm when a TLS
connection is first initialised.

This creates an invariant that the TLS connection always has a valid
key exchange algorithm pointer even if that pointer does not yet
represent the algorithm being used, matching the usage pattern for the
TLS connection's cipher suite and reducing the opportunities for
potential future NULL pointer dereferences.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 19:54:17 +01:00
Michael Brown 8383acf487 [tls] Ensure that broken cipher suites can never be selected
Enforce the invariant that all pointer fields have non-NULL values in
all cipher suites by refusing to ever select a cipher suite that fails
this invariant check.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 19:17:14 +01:00
Michael Brown cc87836a00 [tls] Remove redundant cipher suite name components in debug messages
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 18:52:27 +01:00
Michael Brown 202466d8a6 [tls] Use null handshake digest algorithm for the null cipher suite
A cipher suite's handshake digest algorithm is used only once that
cipher suite has been selected.  The null cipher suite can never be
explicitly selected, and so its handshake digest algorithm can never
be used and is currently left as a NULL pointer.

Set the null cipher suite's handshake digest algorithm to be the null
digest algorithm, to create an invariant that all pointer fields have
non-NULL values in all cipher suites.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 18:13:05 +01:00
Michael Brown 7afbcac103 [tls] Use null MAC digest algorithm for AEAD ciphers
The AEAD cipher suites all have a zero MAC length, since message
authentication is provided by the AEAD cipher itself and the MAC
digest algorithm is never used.

Avoid an unnecessary and unused stack allocation (for the MAC digest
output value) by switching these cipher suites to use the null digest
algorithm instead.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 17:46:55 +01:00
Michael Brown d86e86dff8 [tls] Guarantee that receive data buffer list is non-empty
Since commit 72db146 ("[tls] Split received records over multiple I/O
buffers"), receive record processing has worked by allocating an I/O
buffer list with exactly enough combined tailroom to hold the incoming
record.

A zero-length record is not permitted by the protocol (and is
impossible when a non-plaintext cipher is in use due to the extra
space reserved at the start of the buffer list to accommodate the IV),
but would currently result in an empty I/O buffer list.

Fix by ensuring that at least one buffer is always allocated even for
a zero total length.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-25 00:07:06 +01:00
Michael Brown e97772c34a [tls] Use standalone secure channel and key schedule implementations
The TLS implementation has become too complex to reason about safely,
and needs to be split up into smaller and well-defined units before
adding any further complexity (e.g. to support TLS version 1.3).

Use the standalone secure channel implementation to ensure that the
connection can be used for encrypted communication with a trusted
peer, with the secure channel operations being provided by the
standalone TLS key schedule implementation.

The TLS implementation is now just a protocol engine, and is no longer
responsible for policy decisions on whether or not the connection is
secure.  The TLS code delegates this decision to the underlying secure
channel, by refusing to transmit or receive application data unless
the secure channel has successfully been marked as established.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-24 18:01:51 +01:00
Michael Brown 147ff45f41 [crypto] Add a standalone implementation of the TLS key schedules
The TLS implementation has become too complex to reason about safely,
and needs to be split up into smaller and well-defined units before
adding any further complexity (e.g. to support TLS version 1.3).

Define an abstraction of a TLS key schedule responsible for performing
all cryptographic calculations required by the TLS protocol.  Three
implementations of this abstraction are provided:

  - a TLS version 1.3 key schedule using HKDF

  - a TLS version 1.2 key schedule using PRF based on P_Hash()

  - a TLS version 1.0/1.1 key schedule using PRF based on
    P_MD5()+P_SHA1()

Test cases are included for the HKDF key schedule using the example
known-working handshake traces provided in RFC 8448, and for the
earlier key schedules using manually constructed dummy minimal tests
(with non-functional handshake records) that at least guard against
future regressions.

The separation of concerns will allow the TLS code to become merely a
(still fairly complex) protocol engine, with the responsibility for
all cryptographic calculations being delegated to the key schedule
implementation.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-24 16:45:22 +01:00
Michael Brown 2898206f28 [crypto] Add a generic implementation of a secure channel
The TLS implementation has become too complex to reason about safely,
and needs to be split up into smaller and well-defined units before
adding any further complexity (e.g. to support TLS version 1.3).

Define a generic concept of a secure channel as comprising a pair of
ciphers (one for transmit, one for receive) together with the
cryptographic state required to establish that these ciphers may be
used for encrypted communication with a trusted peer.

The secure channel model is loosely constructed as a generalisation of
TLS minus the protocol specifics, and covers all the expected patterns
of operation for TLS versions 1.1 to 1.3 (including classic RSA key
transport with no forward secrecy, ephemeral key exchange via either
ClientKeyExchange and ServerKeyExchange or via key_share extensions in
ClientHello and ServerHello, and session resumption via either session
IDs, session tickets, or pre-shared keys).

The separation of concerns will allow the TLS code to become merely a
(still fairly complex) protocol engine, with the responsibility for
ensuring the security of the connection being delegated to the secure
channel implementation.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-24 16:45:22 +01:00
Michael Brown 53ac04e1ce [tls] Clean up session management
Define structures to hold session IDs and session tickets, and use
these within the TLS session and connection structures.

Redefine the session ID embedded within the connection structure as
representing the new session ID (if provided), with the existing
session ID now read directly from the session structure.  This reduces
confusion by matching the semantics of the session ticket.

Centralise functions for saving and resuming sessions, to separate
this logic from the protocol parsing code.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-24 10:48:13 +01:00
Michael Brown 5d40abee8a [tls] Clarify purpose of stored key exchange algorithm
Reduce hidden state by making the key exchange algorithm an explicitly
parsed parameter from the ServerKeyExchange record, and storing it
only for the purpose of defining which key exchange algorithm should
be used when sending a ClientKeyExchange.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-24 09:16:01 +01:00
Michael Brown 8958a4a660 [tls] Separate the concepts of key schedule and secure channel
In preparation for delegating the separate but related concepts of a
secure channel and a TLS key schedule, split these into separate data
structures.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-23 22:36:39 +01:00
Michael Brown 6322f50cc8 [tls] Unify handling of ServerKeyExchange and ClientKeyExchange
Restructure the key exchange abstractions to provide a single method
to parse the suite-specific ServerKeyParams structure, with the
signature verification subsequently performed by the caller.

Reduce the suite-specific variation for ClientKeyExchange to a single
parameter that specifies the length of the initial length field, since
this is the only substantive difference between the various mechanisms
as far as the ClientKeyExchange record format is concerned.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-23 22:24:47 +01:00
Michael Brown 350c4e27f5 [crypto] Avoid out-of-bounds read when using uninitialised AES context
Using a cipher or digest algorithm before initialising its context is
not expected to produce any meaningful results, but is expected to be
a safe operation.

The AES context includes the number of rounds, since this varies based
on the key size.  If an uninitialised context is used to perform AES
encryption or decryption, then the code may read beyond the end of the
round key arrays within the context.

This out-of-bounds read is currently reachable only via the use of an
allocated and zeroed but as yet unkeyed TLS cipher context.  The
invalid number of rounds can therefore only ever be zero, which will
result in the number of intermediate rounds being calculated as -2,
which will cause an out-of-bounds read of approximately 64GB of data
(or multiple reads of the full 4GB of a 32-bit address space).

Reading this much address space will almost certainly crash the
system: either by hitting an unmapped page (if paging is enabled), or
by hitting an MMIO region.  In the extremely unlikely event that the
system survives the 64GB read, the decrypted record will immediately
be rejected by TLS for failing to produce a correct authentication tag
or MAC.  There is no viable way that this could be used to extract
confidential information: it could only be used as a denial-of-service
attack.

Fix by ensuring that the number of intermediate rounds is always
calculated as a safe value that cannot overflow the bounds of the
round key arrays.  (The chosen form of the calculation also happens to
ensure that the number of intermediate rounds is always odd, matching
the documented requirement for this to be the case.)

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-23 13:19:04 +01:00
Michael Brown 0ab2277c40 [arm] Add missing condition code clobbers to inline assembly
For x86, GCC will assume that any inline assembly instruction clobbers
the condition code and so it is not necessary to explicitly include
"cc" within the clobber list.

For ARM (both 32-bit and 64-bit), GCC will assume that the condition
code is preserved unless explicitly clobbered.  Most ARM instructions
preserve the condition code by default, and most inline assembly
blocks that modify the condition code already correctly include "cc"
in the clobber list (or as an output operand).

Review all ARM inline assembly and fix the few instances where "cc" is
required but is currently missing from the clobber list.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-21 12:07:23 +01:00
Michael Brown 87a2016ddd [s390x] Add missing condition code clobbers to inline assembly
For x86, GCC will assume that any inline assembly instruction clobbers
the condition code and so it is not necessary to explicitly include
"cc" within the clobber list.

For s390x, GCC will assume that the condition code is preserved unless
explicitly clobbered.  It is extremely rare for GCC to emit code that
actually relies upon this behaviour, but it has been observed to
happen within a series of memcpy() calls with condition-dependent
source addresses.

Fix by adding "cc" to the clobber list on all inline assembly blocks
where the condition code is not guaranteed to be preserved.

It would be possible to use "=@cc" as an output constraint to obtain
the carry result from bigint_add() and bigint_subtract(), but the
instructions generated by GCC to extract the condition code end up
being larger than the single "alcr"/"slbr" that we currently use to
obtain the carry result.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-21 11:47:52 +01:00
Michael Brown e6d0a97c05 [tls] Model classic RSA key transport as a key exchange algorithm
Choose to model key transport as a key exchange algorithm that is
incapable of generating public keys and where the public key size is
zero (implying that the shared secret must be communicated via a means
other than key exchange).

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-11 11:09:33 +01:00
Michael Brown c10c815181 [dns] Add redundant explicit check for CNAME validity
Passing the invalid length returned from a failed call to dns_copy()
in to dns_question() will cause the latter to fail safely, but via a
path that is not obviously safe at first glance.

Check the return value from dns_copy() at the point of use, to reduce
future review noise.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-10 14:41:10 +01:00
Michael Brown b65faaed14 [bitmap] Avoid potential integer overflow when calculating block count
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-10 12:54:22 +01:00
Michael Brown 034ef2bd9a [ata] Use data-transfer buffers for data-in and data-out
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-10 11:51:46 +01:00
Michael Brown 8baf1cda77 [crypto] Correct harmless arithmetic error in Weierstrass curve sizing
The calculation of the number of zero padding bits required to ensure
that relaxed Montgomery multiplication produces a result in the chosen
range is incorrect: the requirement is k=m^2 rather than k=m.

This makes no difference to the code: for both P-256 and P-384, adding
any zero padding bits will cause an extra big integer element to be
used.  This extra element provides 32 (or 64) zero padding bits, which
is many more than are required to ensure that relaxed Montgomery
multiplication produces a result in the chosen range.

Fix the calculation, and update the comments to match.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-09 22:51:42 +01:00
Michael Brown 2fa9f8b1e1 [crypto] Add assertion on HKDF output length
The security proof for HKDF requires each HMAC hash within the
iteration to be computed over distinct input values.  The 8-bit
counter values (ranging from 0x01 to 0xff) provide this formal
guarantee as long as the overall output length is no more than 255
hash blocks.

Even if the counter is allowed to wrap, output is vanishingly unlikely
to repeat since each block's input value also includes the output from
the previous block.  However, this is not a formal guarantee.

RFC 5869 mentions the output length constraint only in passing and in
parentheses.  HKDF is intended to be used to produce small quantities
of key material, and so no realistic consumer will ever exceed the
output length constraint (which is almost 8kB for SHA-256).

There is no point in making this a runtime check.  Doing so would
require every hkdf_expand() call site to include a completely
unnecessary error handling code path, for no real benefit.

Add an assertion to indicate that we are aware of the constraint, and
to reduce future review noise.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-09 10:50:21 +01:00
Michael Brown 719173a3b1 [crypto] Add redundant high-bit clamp to X25519 scalar multiple
The MSB in the scalar multiple is implicitly clamped to zero since the
ladder-based curve point multiplication loop ignores this bit anyway.
However, the missing explicit clamp may confuse reviewers of the code.

Add the explicit clamp to reduce future review noise.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-09 10:20:30 +01:00
Michael Brown e7f0604575 [crypto] Correct harmless arithmetic error in X25519 comments
Step 3 of the x25519 modular multiplication has a marginally tighter
upper bound than is currently claimed by the comments, due to an
arithmetic error caused by the number 19 occurring too often when
writing about this field prime.

Correct the comments to minimise future confusion.  There is no change
required to the code: the overall bound on the step 3 result remains
as previously stated.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-09 01:07:59 +01:00
Michael Brown 257e8faf10 [test] Allow for key exchange tests without an expected public key
Some public key exchange test vector sets provide only the expected
shared secret as an output, without specifying the expected public key
derived from the private key.

Allow key exchange tests to omit the expected public key, so that we
can use these public test vectors without needing to synthesize an
expected public key value.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-08 23:05:28 +01:00
Michael Brown 7e882b7925 [crypto] Add a null key exchange algorithm
Key transport, as required for the classic TLS static RSA pre-master
secret, may be modelled as a key exchange algorithm where the public
key size is zero and the shared secret is constructed unilaterally (to
then be transported via an encrypted channel).

Add a null key exchange algorithm that will fail all key exchange
operations.  The algorithm may be used as a placeholder for consumers
that do not wish to risk forgetting to check for a NULL algorithm
pointer value, and the methods may be used as stubs by algorithms that
do not implement all of the possible key exchange operations (such as
key transport algorithms).

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-08 19:18:36 +01:00
Michael Brown e6e51ccbf1 [build] Move various pointer assignments after their length checks
When performing a length check on untrusted received data, it is
preferable to assign the corresponding typed pointer only after
validating that the length is sufficient to contain the dereferenced
pointer type.  This allows the compiler to catch any unintended
dereferences before the length check has taken place, and so hardens
the code against future potential changes.

This pattern of assigning the pointer only after the corresponding
length check is already fairly widespread, but there are still large
swathes of older code that use the less safe idiom of assigning the
pointer first (generally as part of the variable declaration).

Move an assortment of pointer assignments after their corresponding
length checks, and fix the few harmless premature dereferences that
were discovered in the process (e.g. using a potentially non-existent
IPv4 source address as a debug colour stream identifier).

This is not intended to be a comprehensive update of all such pointer
assignments, merely an improvement of those sites where assignments
are easily identifiable and trivially hardened.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-07 15:58:21 +01:00
Michael Brown 6215a886d2 [usb] Guard against invalid descriptor lengths in USB configurations
A malicious USB device is out of scope for our threat model, but we
already sanity check other descriptor fields, so we should also check
that the reported length of a descriptor contained within a USB device
configuration is adequate for the claimed descriptor type.

Update the two descriptor iterators to skip over descriptors that are
shorter than the length required to contain the iterator type, so that
the loop body can assume that it is safe to dereference any field
within the iterator structure.  Simplify the call sites by integrating
the descriptor type check into the iterator itself, since it fits very
naturally alongside the length check.

Guard against infinite loops by ignoring any descriptors with a length
field that is too short to contain the descriptor header itself.

Validate the descriptor length in usb_endpoint_companion_descriptor(),
which is the only standalone use of usb_next_descriptor() outside of
the two iterators.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-07 14:44:22 +01:00
Michael Brown fe5f845498 [vmbus] Check for packets shorter than their own header length
A malicious hypervisor is out of scope for our threat model, but we
already sanity check other length fields in received packets so we
should also check that the reported header-inclusive length is at
least equal to the reported header length (and thereby avoid a
potential integer underflow).

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-06 16:42:38 +01:00
Michael Brown bfc442ad18 [ucode] Remove harmless read beyond end of malformed equivalence table
If the AMD microcode equivalence table is malformed and is not an
exact multiple of the entry size, then we may read up to two bytes
beyond the end of the allocated image.

The small out-of-bounds read is harmless since the immediately
following code will reject any image with fewer than eight bytes
remaining after the equivalence table (or will harmlessly return
immediately if the out-of-bounds read value was 0x00000000 and no
previous equivalence table entries were present).

Fix by adjusting the loop condition to ignore partial equivalence
table entries.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-06 15:50:13 +01:00
Michael Brown 0f4a37bc3a [doc] Add agent-facing instructions
Add the instructions that Claude developed for itself over the course
of a very interactive week-long security audit of the iPXE codebase.
These instructions are to be used to guide any future use of AI agents
to search for security issues in iPXE.

Agents that follow these instructions are expected to surface only
relevant information, write up suitably minimalistic reports (unlike
the typical unguided AI slop that resulted in iPXE's current "(Ab)use
of AI" policy), and guide submission through the appropriate channels
that have been set up and documented in the security policy.  Any
AI-authored reports are directed towards the "ipxe/aipxe" sandbox
repository, which exists to provide a clear separation between
human-generated and AI-generated content.

Given that repeated passes with Claude Opus 4.8 (and a cross-check
with Claude Fable) have converged to a clean state, it is expected
that publishing these instructions will lead to at most a trickle of
submissions, and that any such submissions should end up being
genuinely useful.

These instructions were written by Claude (with many hours of guidance
and refinement) and have not been modified, on the basis that an AI
agent knows best about what documentation it will itself find useful.
Unnecessary duplication has been avoided by documenting the key points
(e.g. bounds contracts) within the code's own Doxygen comments for
reference by both humans and agents, and ensuring that Claude's own
instructions refer and defer to this authoritative documentation.

Claude has not authored any code that was committed as part of this
week-long project.  The AI agent instructions added by this commit
remain the only AI-authored content present in the tree.  I have set
myself as the commit author (with an appropriate Authored-by credit
for Claude), written this commit message myself, and added my own
signoff, to confirm that I am the human owner taking long-term
responsibility for this contribution, regardless of its origin.

Authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-06 12:47:53 +01:00
Michael Brown d00df0822b [doc] Add security policy document
With suitable guidance, AI agents such as Claude Code are capable of
scanning effectively for potential vulnerabilities, and reporting them
in a concise and actionable format.

These tools are now widely available to malicious actors, and so any
vulnerabilities that they are capable of finding must be fixed now
before they are inevitably found and potentially exploited.

The recent batch of commits over the past week closes all potential
vulnerabilities that were detectable by either Opus 4.8 or Fable in
multiple passes over the code.  No serious security impact was found,
and there is nothing that would merit a UEFI Secure Boot revocation.

A concrete threat model is now documented, along with the explicit
bounds contracts for several internal APIs (such as ASN.1 parsing and
I/O buffer pointer manipulation).  Some entire classes of nominal
defect (e.g. technically undefined behaviour arising from constant
left shifts into the sign bit) have been eliminated.  False positives
that were raised several times and that could not be silenced through
reporting guidelines were fixed in the code, even when the code change
had no real-world impact.  It is now possible to ask an appropriately
instructed AI agent to search for vulnerabilities in the iPXE codebase
and to be reasonably confident that anything that it reports is worth
investigating further.

Add a security policy to formally document the expectations upon both
humans and AI agents in terms of reporting potential vulnerabilities,
and update the contribution guidelines to grant a limited exception to
the blanket ban on AI-generated text.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-06 00:01:56 +01:00
Michael Brown d3f96b7cad [ci] Add a workflow to trigger synchronisation in forks
Add a workflow that dispatches the synchronisation workflow in a
repository-defined list of downstream forks.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-05 16:59:39 +01:00
Michael Brown 6084600b47 [ci] Add a workflow to run in forks to synchronise from the upstream
Add a workflow that can be dispatched within a fork to synchronise it
from the upstream repository.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-05 15:26:05 +01:00
Michael Brown 36c25549c8 [settings] Fix limited out-of-bounds read in fetch_numeric_setting()
The code in fetch_numeric_setting() reads the setting value into a
local fixed-size buffer but then passes the full setting length to
numeric_setting_value().  If the setting length exceeds the size of
the fixed-size buffer, then numeric_setting_value() will continue to
read bytes from the stack.

The number of bytes read is constrained: numeric_setting_value() will
exit with -ERANGE as soon as the value being constructed exceeds the
range of an unsigned long.  The existence of a return address on the
stack thus provides an upper bound on how far numeric_setting_value()
can read before terminating with an error.

Creating a setting with a length of more than an unsigned long is
trivial, for example:

  set thing:hexraw 00000000000000000000000000000000

However, the out-of-bounds read can be reached only via calls to the
fetch_[u]int[z]_setting() family of internal helper functions.
Reading the setting in a script via e.g. ${thing:uint32} goes via a
different code path that does not use a fixed-length buffer.

The fetch_[u]int[z]_setting() functions are called from only a few
places.  Most uses are for boolean flags or bit masks.  A few are
genuinely used as numeric values: the settings mechanism itself reads
and uses the "priority" setting, the network core reads the "mtu"
setting, and the SAN boot mechanism reads the drive number and retry
count.

An extremely determined attacker could potentially obtain up to eight
bytes of information from the stack (in a 64-bit build) by, for
example, creating two sibling settings blocks where one has an
overlength "priority" setting value, and then repeatedly manipulating
the priority in the other settings block and testing to see which
block ends up with the higher priority.  The information that could be
obtained in this way is limited to the temporary values stored on the
stack by fetch_numeric_setting() itself, along with its own return
address.  None of this information is security-sensitive, and so any
information leakage is a mere curiosity.

Fix by allocating a temporary copy within fetch_numeric_setting()
instead of using a fixed-size buffer.  This has the downside of
introducing an otherwise unnecessary memory allocation (which could
potentially itself fail), but guarantees consistency with other
numeric interpretations of setting values.  (The alternative approach
of rejecting overlength setting values would introduce a potential
inconsistency between the value returned by fetch_numeric_setting()
and the value obtained by formatting a setting using a numeric setting
type, or by numerating the setting.)

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-05 13:48:16 +01:00
Michael Brown 092ab54ebc [ipv4] Remove harmless but technically undefined left shift
A DHCP static route option is capable of encoding an invalid subnet
mask width of greater than 32 bits.  This leads to a technically
undefined left shift when calculating the 32-bit subnet mask.

There is no security impact of this undefined shift: the only possible
outcome is that the subnet mask for the improperly defined static
route ends up holding an invalid value.

Fix by checking the range before performing the shift, to eliminate
future reporting noise.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 18:37:12 +01:00
Michael Brown a1992fedfa [doc] Document the threat model relevant to iPXE
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 15:48:34 +01:00
Michael Brown 5e706ff4c5 [fcoe] Add assorted length checks
Add an assortment of missing length checks that can currently result
in reads of uninitialised data from within the Ethernet frame padding
region of a received I/O buffer.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 13:51:08 +01:00
Michael Brown 894a7e04be [efi] Avoid reading beyond end of command line
The EFI command line is not necessarily terminated with a wNUL
character.  We currently use snprintf() with an output buffer size to
constrain the write to the correct size and ensure that a NUL
terminator exists (as required for the image data), but nothing
prevents snprintf() from continuing to pointlessly read beyond the end
of the wide-character command line until it happens to encounter a
wNUL somewhere.

Fix by creating a temporary wNUL-terminated copy of the EFI command
line and then converting that (in situ) to ASCII.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 13:17:53 +01:00
Michael Brown ba98f2e150 [efi] Guard against invalid IpCnt values in the PXE IP address filter
A caller that places an invalid value in the IpCnt field would cause
iPXE to read beyond the end of the IpList array.

This has no meaningful security impact: there is no out-of-bounds
write, and a caller with the ability to place an invalid value in the
IpCnt field would already have to be a Secure Boot signed binary (if
Secure Boot is enabled).

Fix by limiting the traversal of IpList to the lower of IpCnt or the
array size, to reduce unwanted noise from security reviewers.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 12:58:25 +01:00
Michael Brown 45af75d2af [efi] Treat invalid device path components as ending the path
EFI device paths generally have no externally defined length: the only
way to calculate the length is to scan the device path itself (and
therefore to implicitly assume that the path is valid).

There is no way to guard against a malformed device path (absent the
atypical existence of an external length), but we can at least prevent
infinite loops from a device path component that encodes a zero
length.

Treat any device path component with a length too short to contain the
device path header as ending the device path.  This does not prevent
invalid device paths from being accepted, but it does at least guard
against a silent system hang from an infinite loop, and ensures that
callers may safely subtract the length of the device path header from
the length of the path component without underflowing.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 12:21:46 +01:00
Michael Brown 5de611be4a [efi] Fix check for well-formed device paths
EFI device paths generally have no externally defined length: the only
way to calculate the length is to scan the device path itself (and
therefore to implicitly assume that the path is valid).

The EFI load option structure does have an externally defined length
field, and we currently attempt to validate against this.  The
validation logic is missing a crucial step which renders it
ineffective: the overall effect is essentially equivalent to trusting
that the system's configured load option structures are well-formed.
(This is a reasonable assumption: the length check exists primarily as
a defence against external bugs, and an attacker with the ability to
change the system load options has already compromised the system.)

Fix by updating the remaining length correctly as we traverse the
device path.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 12:21:41 +01:00
Michael Brown eaf987535e [elf] Avoid harmless integer overflows in image length checks
Fix the checks against reading beyond the image length when executing
an ELF image.

As with the equivalent commit 979c86f ("[nbi] Avoid harmless integer
overflows in image length checks"), this change has absolutely no
security impact: an ELF image will obtain control of the system in
ring 0 anyway, and so a "malicious" ELF image with malformed length
fields cannot do anything that it would not already be able to do
simply by being executed.  However, fixing these harmless integer
overflows costs very little and reduces unwanted noise from security
reviewers.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 10:54:58 +01:00
Michael Brown 3812a69c7f [uhci] Fix descriptor count for zero-length stream transfers
The descriptor count for a zero-length stream transfer with no
explicit terminating zero-length packet is currently calculated
incorrectly as requiring zero descriptors.  This will cause
uhci_enqueue() to attempt to allocate a zero-length block of transfer
descriptors, which will fail and return -ENOMEM.

There is no internal code path within iPXE that can ever submit a
zero-length stream transfer without an explicit terminating
zero-length packet.  This condition is reachable only via the
EFI_USB_IO_PROTOCOL interface that we expose on UEFI platforms to
allow existing firmware drivers to reconnect after we take control of
the host controller.

Fix by ensuring that the descriptor count is set to one for a
zero-length stream transfer with no explicit terminating zero-length
packet, as is already done for EHCI and XHCI.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 09:59:02 +01:00
Michael Brown 897c87a8fa [velocity] Correct direction of endianness conversion
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 08:46:24 +01:00
Michael Brown 3b06e419a9 [uhci] Add missing little-endian conversion
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 08:43:01 +01:00
Michael Brown 8cd4a0c6cd [intelxl] Add missing little-endian conversions
Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 08:38:10 +01:00
Michael Brown 1e8f4fed41 [build] Enable strict shift overflow warnings
Left shifts into the sign bit are often reported as potential
undefined behaviour by automated tools, which distracts from real
issues.

Now that all offending constant left shifts have been eliminated from
the codebase, enable -Wshift-overflow=2 to ensure that such shifts
cannot be reintroduced in future.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
2026-08-04 00:19:00 +01:00