[Bug]: Out-of-Bounds Memory Access in Percent-Decoding Routine

[florath]

Is there an existing issue for this?

• I have searched existing issues

Bug Type

• Firmware
• Tool
• Unit Test

Code first?

• Yes

What packages are impacted?

NetworkPkg

Which targets are impacted by this bug?

DEBUG, NO-TARGET, NOOPT, RELEASE

Current Behavior

It was discovered that the percent-decoding loop in the boot file URL extraction function accesses memory beyond the allocated buffer. When a '%' character is encountered near the end of the string, the code assumes that at least two characters follow it. This assumption leads to reading from and writing to *(BootFileNamePtr + 3) without validating the existence of sufficient characters.

PxeBcDhcp6.c:553ff

    //
    // Decode percent-encoding in boot file name.
    //
    while (*BootFileNamePtr != '\0') {
      if (*BootFileNamePtr == '%') {
        TmpChar               = *(BootFileNamePtr+ 3);
        *(BootFileNamePtr+ 3) = '\0';
        *BootFileName         = (UINT8)AsciiStrHexToUintn ((CHAR8 *)(BootFileNamePtr + 1));
        BootFileName++;
        *(BootFileNamePtr+ 3) = TmpChar;
        BootFileNamePtr      += 3;
      } else {
        *BootFileName = *BootFileNamePtr;
        BootFileName++;
        BootFileNamePtr++;
      }
    }

    *BootFileName = '\0';
  }

Expected Behavior

The function should verify that at least two characters follow the '%' before attempting to decode a percent-encoded sequence. Incomplete sequences should be handled gracefully—either by treating the '%' as a literal or by rejecting the input.

Steps To Reproduce

1. Provide a boot file URL where a '%' appears as the penultimate or last character (e.g., tftp://[::1]/filename%1 or tftp://[::1]/filename%).
2. Call the PxeBcExtractBootFileUrl function.
3. Observe that the code attempts to access memory beyond the allocated buffer.

Build Environment

none - Code Review after a crash of my BIOS

Version Information

Current head.
Commit: 1f1182c396466300ad6659c42b517542c61706d9

Urgency

High

Are you going to fix this?

Someone else needs to fix it

Do you need maintainer feedback?

Maintainer feedback requested

Anything else?

Here an implementation that checks if there are at least two characters after the '%'. This could possible replace lines 543ff of PxeBcDhcp6.c.

The following code ignores '%' when there are at least not two additional characters. Another implementation could be to return an error in this case.

//
// Decode percent-encoding in boot file name.
//
while (*BootFileNamePtr != '\0') {
  if (*BootFileNamePtr == '%') {
    if (*(BootFileNamePtr + 1) == '\0' || *(BootFileNamePtr + 2) == '\0') {
      *BootFileName = *BootFileNamePtr;
      BootFileName++;
      BootFileNamePtr++;
    } else {
      CHAR8 hexStr[3];
      hexStr[0] = *(BootFileNamePtr + 1);
      hexStr[1] = *(BootFileNamePtr + 2);
      hexStr[2] = '\0';
      *BootFileName = (UINT8)AsciiStrHexToUintn(hexStr);
      BootFileName++;
      BootFileNamePtr += 3;
    }
  } else {
    *BootFileName = *BootFileNamePtr;
    BootFileName++;
    BootFileNamePtr++;
  }
}

*BootFileName = '\0';

[lgao4]

@Zclarkwilliams , could you help confirm this problem and the proposed change?

[florath]

Today I found time to run some tests. Using this line (I changed the IP address):

option dhcp6.bootfile-url "tftp://[2bf3:769d:0:2081:7:111:0:231]/grubnetx64.efi%4";

results in this filename (tcpdump on port 69 on the server).

[udp sum ok] TFTP, length 71, RRQ "grubnetx64.efi^DM-/M-/M-/M-/ptalM-/M-/M-/M-/`" octet tsize 0 blksize 1228 windowsize 4

The console gets some greyish background and is unresponsive (does not accept any keyboard input).

The only way out seams to be a reboot.

Update: I just saw, that there is a alarm in the BMC

System BIOS has halted