fix: harden share bot uploads
This commit is contained in:
+32
-3
@@ -2,7 +2,10 @@ import { timingSafeEqual } from "node:crypto";
|
||||
import { cdnUrl } from "@/lib/cdn-images";
|
||||
import { ensureDiscordUser } from "@/lib/auth/discord-user";
|
||||
import { getCanonicalUrl, getShareBotSecret } from "@/lib/config/server";
|
||||
import { getDiscordMemberProfile } from "@/lib/discord/discord";
|
||||
import {
|
||||
DiscordProfileLookupError,
|
||||
getDiscordSenderProfile,
|
||||
} from "@/lib/discord/discord";
|
||||
import { createShare } from "@/lib/share/create";
|
||||
import { ShareHttpError, shareErrorResponse } from "@/lib/share/http-error";
|
||||
import { enforceShareRateLimit } from "@/lib/share/rate-limit";
|
||||
@@ -44,7 +47,25 @@ export async function POST(request: Request) {
|
||||
windowSeconds: 60 * 60,
|
||||
});
|
||||
|
||||
const formData = await request.formData();
|
||||
let formData: FormData;
|
||||
try {
|
||||
formData = await request.formData();
|
||||
} catch {
|
||||
throw new ShareHttpError("Malformed multipart form data", 400);
|
||||
}
|
||||
for (const field of [
|
||||
"displayName",
|
||||
"avatarUrl",
|
||||
"authorName",
|
||||
"authorAvatarUrl",
|
||||
]) {
|
||||
if (formData.has(field)) {
|
||||
throw new ShareHttpError(
|
||||
`Profile field ${field} must not be supplied`,
|
||||
400,
|
||||
);
|
||||
}
|
||||
}
|
||||
const file = formData.get("file");
|
||||
const senderDiscordId = formData.get("senderDiscordId");
|
||||
if (!(file instanceof File)) {
|
||||
@@ -62,7 +83,15 @@ export async function POST(request: Request) {
|
||||
validateOptionalImage(formData.get("image")),
|
||||
]);
|
||||
const description = validateShareDescription(formData.get("description"));
|
||||
const profile = await getDiscordMemberProfile(senderDiscordId);
|
||||
let profile;
|
||||
try {
|
||||
profile = await getDiscordSenderProfile(senderDiscordId);
|
||||
} catch (error) {
|
||||
if (error instanceof DiscordProfileLookupError) {
|
||||
throw new ShareHttpError("Discord profile lookup is unavailable", 503);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (!profile) {
|
||||
throw new ShareHttpError(
|
||||
"Discord sender was not found in the configured guild",
|
||||
|
||||
@@ -24,6 +24,18 @@ export interface DiscordMemberProfile {
|
||||
roles: DiscordRole[];
|
||||
}
|
||||
|
||||
export type DiscordSenderProfile = Pick<
|
||||
DiscordMemberProfile,
|
||||
"id" | "username" | "globalName" | "avatarUrl"
|
||||
>;
|
||||
|
||||
export class DiscordProfileLookupError extends Error {
|
||||
constructor(message = "Discord profile lookup is unavailable") {
|
||||
super(message);
|
||||
this.name = "DiscordProfileLookupError";
|
||||
}
|
||||
}
|
||||
|
||||
interface CacheEntry<T> {
|
||||
data: T;
|
||||
expiresAt: number;
|
||||
@@ -213,3 +225,49 @@ export async function getDiscordMemberProfile(discordId: string): Promise<Discor
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function getDiscordSenderProfile(
|
||||
discordId: string,
|
||||
): Promise<DiscordSenderProfile | null> {
|
||||
try {
|
||||
const { token, guildId } = getDiscordServerConfig();
|
||||
const response = await fetch(
|
||||
`https://discord.com/api/v10/guilds/${encodeURIComponent(guildId)}/members/${encodeURIComponent(discordId)}`,
|
||||
{
|
||||
headers: { Authorization: `Bot ${token}` },
|
||||
cache: "no-store",
|
||||
signal: AbortSignal.timeout(5000),
|
||||
},
|
||||
);
|
||||
|
||||
if (response.status === 404) return null;
|
||||
if (!response.ok) throw new DiscordProfileLookupError();
|
||||
|
||||
const member = (await response.json()) as {
|
||||
user?: {
|
||||
id?: string;
|
||||
username?: string;
|
||||
global_name?: string | null;
|
||||
avatar?: string | null;
|
||||
};
|
||||
};
|
||||
const user = member.user;
|
||||
if (!user?.id || !user.username || user.id !== discordId) {
|
||||
throw new DiscordProfileLookupError("Discord returned an invalid sender profile");
|
||||
}
|
||||
|
||||
const avatarUrl = user.avatar
|
||||
? `https://cdn.discordapp.com/avatars/${user.id}/${user.avatar}.${user.avatar.startsWith("a_") ? "gif" : "png"}?size=128`
|
||||
: null;
|
||||
|
||||
return {
|
||||
id: user.id,
|
||||
username: user.username,
|
||||
globalName: user.global_name ?? null,
|
||||
avatarUrl,
|
||||
};
|
||||
} catch (error) {
|
||||
if (error instanceof DiscordProfileLookupError) throw error;
|
||||
throw new DiscordProfileLookupError();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,8 +24,9 @@ describe("share validation", () => {
|
||||
throw new Error("Expected blank text to fail");
|
||||
} catch (error) {
|
||||
expect(error).toBeInstanceOf(ShareHttpError);
|
||||
expect((error as ShareHttpError).status).toBe(422);
|
||||
expect((error as ShareHttpError).status).toBe(400);
|
||||
}
|
||||
expect(() => validateShareText("before\0after")).toThrow("NUL");
|
||||
});
|
||||
|
||||
test("measures the text limit in UTF-8 bytes", () => {
|
||||
@@ -65,7 +66,12 @@ describe("share validation", () => {
|
||||
type: "text/plain",
|
||||
})
|
||||
)
|
||||
).rejects.toMatchObject({ status: 422 });
|
||||
).rejects.toMatchObject({ status: 400 });
|
||||
await expect(
|
||||
validateTextFile(
|
||||
new File(["before\0after"], "nul.txt", { type: "text/plain" })
|
||||
)
|
||||
).rejects.toMatchObject({ status: 400 });
|
||||
});
|
||||
|
||||
test("validates image contents instead of trusting the MIME type", async () => {
|
||||
|
||||
@@ -18,7 +18,10 @@ export function validateShareText(content: unknown) {
|
||||
throw new ShareHttpError("Text is required", 400);
|
||||
}
|
||||
if (!content.trim()) {
|
||||
throw new ShareHttpError("Text cannot be blank", 422);
|
||||
throw new ShareHttpError("Text cannot be blank", 400);
|
||||
}
|
||||
if (content.includes("\0")) {
|
||||
throw new ShareHttpError("Text cannot contain NUL bytes", 400);
|
||||
}
|
||||
if (Buffer.byteLength(content, "utf8") > MAX_SHARE_TEXT_BYTES) {
|
||||
throw new ShareHttpError("Text must be 5 MiB or smaller", 413);
|
||||
@@ -55,7 +58,7 @@ export async function validateTextFile(file: File) {
|
||||
throw new ShareHttpError("Text file must be 5 MiB or smaller", 413);
|
||||
}
|
||||
if (file.size === 0) {
|
||||
throw new ShareHttpError("Text file cannot be empty", 422);
|
||||
throw new ShareHttpError("Text file cannot be empty", 400);
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -64,7 +67,7 @@ export async function validateTextFile(file: File) {
|
||||
return validateShareText(content);
|
||||
} catch (error) {
|
||||
if (error instanceof ShareHttpError) throw error;
|
||||
throw new ShareHttpError("Text file must contain valid UTF-8", 422);
|
||||
throw new ShareHttpError("Text file must contain valid UTF-8", 400);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user