Skip to content

Commit c2d4db5

Browse files
committed
fix: restrict device icon data URI parsing to prevent path traversal
The base64 image regex used a greedy `.+` for the extension capture, letting values like `data:image/png/../../foo;base64,...` flow into path.join() in saveBase64DeviceIcon and produce arbitrary file writes via bridge/request/device/options. Anchor the regex, constrain the extension and data character classes to safe sets, and reject extensions outside an explicit allowlist. Includes a regression test for non-allowlisted image extensions via bridge/request/device/options.
1 parent 3785e4a commit c2d4db5

2 files changed

Lines changed: 23 additions & 2 deletions

File tree

lib/util/utils.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import type {Zigbee2MQTTAPI, Zigbee2MQTTResponse, Zigbee2MQTTResponseEndpoints,
1010

1111
import data from "./data";
1212

13-
const BASE64_IMAGE_REGEX = /data:image\/(?<extension>.+);base64,(?<data>.+)/;
13+
const BASE64_IMAGE_REGEX = /^data:image\/(?<extension>[A-Za-z0-9]+);base64,(?<data>[A-Za-z0-9+/=]+)$/;
14+
const ALLOWED_ICON_EXTENSIONS = new Set(["png", "jpg", "jpeg", "gif", "webp", "svg", "bmp"]);
1415

1516
export const DEFAULT_BIND_GROUP_ID = 901;
1617

@@ -386,8 +387,14 @@ function matchBase64File(value: string | undefined): {extension: string; data: s
386387
}
387388

388389
function saveBase64DeviceIcon(base64Match: {extension: string; data: string}): string {
390+
const extension = base64Match.extension.toLowerCase();
391+
392+
if (!ALLOWED_ICON_EXTENSIONS.has(extension)) {
393+
throw new Error(`Unsupported icon extension '${base64Match.extension}'`);
394+
}
395+
389396
const md5Hash = crypto.createHash("md5").update(base64Match.data).digest("hex");
390-
const fileSettings = `device_icons/${md5Hash}.${base64Match.extension}`;
397+
const fileSettings = `device_icons/${md5Hash}.${extension}`;
391398
const file = path.join(data.getPath(), fileSettings);
392399
fs.mkdirSync(path.dirname(file), {recursive: true});
393400
fs.writeFileSync(file, base64Match.data, {encoding: "base64"});

test/extensions/bridge.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3442,6 +3442,20 @@ describe("Extension: Bridge", () => {
34423442
);
34433443
});
34443444

3445+
it("Should reject icon data URI with disallowed extension", async () => {
3446+
mockMQTTPublishAsync.mockClear();
3447+
const malicious = "data:image/exe;base64,QUFB";
3448+
mockMQTTEvents.message("zigbee2mqtt/bridge/request/device/options", stringify({options: {icon: malicious}, id: "bulb"}));
3449+
await flushPromises();
3450+
3451+
expect(mockMQTTPublishAsync).toHaveBeenCalledWith(
3452+
"zigbee2mqtt/bridge/response/device/options",
3453+
expect.stringContaining("Unsupported icon extension 'exe'"),
3454+
{},
3455+
);
3456+
expect(settings.getDevice("bulb").icon).toBeUndefined();
3457+
});
3458+
34453459
it("Should allow to remove device option", async () => {
34463460
mockMQTTPublishAsync.mockClear();
34473461
settings.set(["devices", "0x000b57fffec6a5b2", "qos"], 1);

0 commit comments

Comments
 (0)