Skip to content

fix(weixin): confirm the WEBP signature, not just the RIFF prefix #5285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Jun 18, 2026
Merged

fix(weixin): confirm the WEBP signature, not just the RIFF prefix #5285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions packages/channels/weixin/src/send.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,43 @@ describe('detectImageMime', () => {
expect(detectImageMime(buf)).toBe('image/gif');
});

it('detects WebP magic bytes (RIFF)', () => {
const buf = Buffer.from([0x52, 0x49, 0x46, 0x46]);
it('detects WebP magic bytes (RIFF....WEBP)', () => {
const buf = Buffer.from([
0x52,
0x49,
0x46,
0x46, // "RIFF"
0x1a,
0x00,
0x00,
0x00, // file size (little-endian)
0x57,
0x45,
0x42,
0x50, // "WEBP"
]);

@wenshao wenshao Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The old test used a 4-byte RIFF buffer which implicitly covered the short-buffer path. The replacement uses a full 12-byte header, so there's no longer a test asserting that a truncated RIFF buffer (< 12 bytes) falls through gracefully rather than crashing. Consider adding:

it('does not crash on a truncated RIFF buffer (< 12 bytes)', () => {
  const buf = Buffer.from([0x52, 0x49, 0x46, 0x46]); // only 4 bytes
  expect(() => detectImageMime(buf)).toThrow('Unrecognized image format');
});

This locks in the graceful fall-through behavior so future refactors cannot silently break it.

— qwen3.7-max via Qwen Code /review

expect(detectImageMime(buf)).toBe('image/webp');
});

it('does not misidentify a non-WebP RIFF container (e.g. WAV) as WebP', () => {
// WAV is also a RIFF container; only bytes 8-11 distinguish it from WebP.
const buf = Buffer.from([
0x52,
0x49,
0x46,
0x46, // "RIFF"
0x24,
0x00,
0x00,
0x00, // file size
0x57,
0x41,
0x56,
0x45, // "WAVE", not "WEBP"
]);
expect(() => detectImageMime(buf)).toThrow('Unrecognized image format');
});

it('detects JPEG magic bytes', () => {
const buf = Buffer.from([0xff, 0xd8, 0xff]);
expect(detectImageMime(buf)).toBe('image/jpeg');
Expand Down
8 changes: 7 additions & 1 deletion packages/channels/weixin/src/send.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,17 @@ export function detectImageMime(data: Buffer): string {
if (data[0] === 0x47 && data[1] === 0x49 && data[2] === 0x46) {
return 'image/gif';
}
// WebP is a RIFF container, so the "RIFF" prefix alone is not enough — WAV and
// AVI share it. The bytes at offset 8-11 must spell "WEBP" to confirm the type.
if (
data[0] === 0x52 &&
data[1] === 0x49 &&
data[2] === 0x46 &&
data[3] === 0x46
data[3] === 0x46 &&

@wenshao wenshao Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The WebP check accesses data[8] through data[11] without an explicit length guard. This works in practice — Buffer[i] returns undefined for out-of-bounds indices, and undefined === 0x57 is false, so short buffers fall through safely. However, this relies on implicit JS behavior rather than an explicit check. A future refactor (e.g., switching to data.readUInt32BE(8)) could silently break this.

Consider adding an explicit guard:

Suggested change
data[3] === 0x46 &&
if (
data.length >= 12 &&
data[0] === 0x52 &&
data[1] === 0x49 &&
data[2] === 0x46 &&
data[3] === 0x46 &&
data[8] === 0x57 &&
data[9] === 0x45 &&
data[10] === 0x42 &&
data[11] === 0x50

— qwen3.7-max via Qwen Code /review

data[8] === 0x57 &&
data[9] === 0x45 &&
data[10] === 0x42 &&
data[11] === 0x50
) {
return 'image/webp';
}
Expand Down
Loading