Patina now has strict UEFI string types (Char8Str/Char16Str, Char8String/Char16String, Char8Array<N>/Char16Array<N>) (see #1657) that validate their invariants on construction. For CHAR16 that means the data must be real UCS-2 which means that it is NUL terminated, has no interior NUL, and does not contain UTF-16 surrogate code units (0xD800..=0xDFFF). In those types, construction fails with StringError::NotUcs2, InteriorNul, or MissingNulTerminator when those invariants are violated.
It is likely that CHAR16 data will reach Patina that does not originate in Patina. For example, across C FFI boundaries and out of non-volatile storage (e.g. UEFI variable names stored on SPI flash) that were written by C code with no UCS-2 enforcement at all. EDK II and most existing firmware treat CHAR16 as an opaque UINT16[] and compare it byte for byte. Nothing explicitly guarantees the values are valid UCS-2. So, Patina will inevitably be handed CHAR16 buffers that originate outside its boundaries, that may be malformed.
It is very unlikely that an issue will occur in real-world usage scenarios, but the possibility needs to be accounted for.
This issue tracks deciding what happens at boundaries when that occurs. Then, defining a policy and auditing code usage to confirm it aligns with the policy.
The security concern
The concern is mostly around what rejecting a bad string does to the operations around it.
As an example: During GetNextVariableName() enumeration, the variable store is walked in order, returning one name at a time. If Patina wraps each returned name in a Char16Str and treats a validation failure as a hard error that stops the walk, then a single malformed name aborts the entire enumeration. Every legitimate variable that is after the bad entry becomes inaccessible.
An attacker (or even benign non-conforming firmware) who can get one variable name containing a lone surrogate onto flash can therefore hide later variables. Depending on what is stored after it, that can mean hiding security relevant state, breaking policy that iterates the store, or a plain denial of service against variable access. The same pattern can apply in other places where Patina iterates externally supplied UEFI string data.
A second issue is panics. Any path that constructs a strict type with unwrap/expect could inadvertently cause a panic from malformed data. Guidance here is that runtime boundary data must go through the try_* / Result returning APIs.
Invariants in scope
- Surrogate code units
0xD800..=0xDFFF (not valid UCS-2). This includes lone surrogates and any
well-formed UTF-16 surrogate pair, since UCS-2 has no concept of pairs and cannot represent code
points above U+FFFF.
- Interior NUL before the terminator.
- Missing NUL terminator (for the terminated constructors).
- Odd byte length when decoding from a little-endian byte buffer.
Boundaries that need a defined policy
These are the boundaries that come to mind:
- C FFI input:
Char16Str::from_ptr / from_ptr_max and the from_units* constructors already return
Result. Every caller across the ABI needs to handle the error rather than unwrap it. A general rule likely
should be that boundary construction never uses a panicking constructor.
- Non-volatile storage reads: Variable names, load options, boot entries, and similar CHAR16 payloads
read from flash. Code should handle the "bad string" scenario enumerating from these sources.
- Comparison and lookup: EDK II compares CHAR16 names as raw
UINT16[]. If Patina normalizes, replaces,
or lossily decodes a name before comparing, it could fail to match (or fail to delete) a variable that a C
consumer can still see by raw bytes. The identity of names should be preserved even when the bytes are not
valid UCS-2 so the overall operation works as expected.
Options to consider
- Skip and continue for sequence/enumeration operations, so one malformed element cannot hide
the rest. Probably paired with a warn! and a counter so we can tell it is happening.
- Lossy view for display and logging only, never for identity, comparison, or persistence.
- Raw passthrough for operations that only need to move or match opaque bytes. These arguably
should operate on &[u16] and never require a validated Char16Str at all.
In the end, the likely usage will be a mix strict where Patina creates data while being tolerant and non-aborting where Patina consumes external data.
Patina now has strict UEFI string types (
Char8Str/Char16Str,Char8String/Char16String,Char8Array<N>/Char16Array<N>) (see #1657) that validate their invariants on construction. For CHAR16 that means the data must be real UCS-2 which means that it is NUL terminated, has no interior NUL, and does not contain UTF-16 surrogate code units (0xD800..=0xDFFF). In those types, construction fails withStringError::NotUcs2,InteriorNul, orMissingNulTerminatorwhen those invariants are violated.It is likely that CHAR16 data will reach Patina that does not originate in Patina. For example, across C FFI boundaries and out of non-volatile storage (e.g. UEFI variable names stored on SPI flash) that were written by C code with no UCS-2 enforcement at all. EDK II and most existing firmware treat CHAR16 as an opaque
UINT16[]and compare it byte for byte. Nothing explicitly guarantees the values are valid UCS-2. So, Patina will inevitably be handed CHAR16 buffers that originate outside its boundaries, that may be malformed.It is very unlikely that an issue will occur in real-world usage scenarios, but the possibility needs to be accounted for.
This issue tracks deciding what happens at boundaries when that occurs. Then, defining a policy and auditing code usage to confirm it aligns with the policy.
The security concern
The concern is mostly around what rejecting a bad string does to the operations around it.
As an example: During
GetNextVariableName()enumeration, the variable store is walked in order, returning one name at a time. If Patina wraps each returned name in aChar16Strand treats a validation failure as a hard error that stops the walk, then a single malformed name aborts the entire enumeration. Every legitimate variable that is after the bad entry becomes inaccessible.An attacker (or even benign non-conforming firmware) who can get one variable name containing a lone surrogate onto flash can therefore hide later variables. Depending on what is stored after it, that can mean hiding security relevant state, breaking policy that iterates the store, or a plain denial of service against variable access. The same pattern can apply in other places where Patina iterates externally supplied UEFI string data.
A second issue is panics. Any path that constructs a strict type with
unwrap/expectcould inadvertently cause a panic from malformed data. Guidance here is that runtime boundary data must go through thetry_*/Resultreturning APIs.Invariants in scope
0xD800..=0xDFFF(not valid UCS-2). This includes lone surrogates and anywell-formed UTF-16 surrogate pair, since UCS-2 has no concept of pairs and cannot represent code
points above
U+FFFF.Boundaries that need a defined policy
These are the boundaries that come to mind:
Char16Str::from_ptr/from_ptr_maxand thefrom_units*constructors already returnResult. Every caller across the ABI needs to handle the error rather than unwrap it. A general rule likelyshould be that boundary construction never uses a panicking constructor.
read from flash. Code should handle the "bad string" scenario enumerating from these sources.
UINT16[]. If Patina normalizes, replaces,or lossily decodes a name before comparing, it could fail to match (or fail to delete) a variable that a C
consumer can still see by raw bytes. The identity of names should be preserved even when the bytes are not
valid UCS-2 so the overall operation works as expected.
Options to consider
the rest. Probably paired with a
warn!and a counter so we can tell it is happening.should operate on
&[u16]and never require a validatedChar16Strat all.In the end, the likely usage will be a mix strict where Patina creates data while being tolerant and non-aborting where Patina consumes external data.