From 00fd9b03d499ab412e521233c0e853cbd64d34c4 Mon Sep 17 00:00:00 2001 From: Andrea Vos Date: Fri, 14 Jun 2024 18:46:23 +0200 Subject: [PATCH 1/2] (security) proxy favicons to prevent potential leakage of users' IPs --- migrations/078-links-favicon-cache.sql | 6 +++ server/analyseLinks.js | 9 ++++- server/avatarCopy.ts | 39 ------------------ server/imageCopy.ts | 55 ++++++++++++++++++++++++++ server/routes/profile.js | 2 +- server/routes/user.js | 4 +- 6 files changed, 72 insertions(+), 43 deletions(-) create mode 100644 migrations/078-links-favicon-cache.sql delete mode 100644 server/avatarCopy.ts create mode 100644 server/imageCopy.ts diff --git a/migrations/078-links-favicon-cache.sql b/migrations/078-links-favicon-cache.sql new file mode 100644 index 000000000..3b844e299 --- /dev/null +++ b/migrations/078-links-favicon-cache.sql @@ -0,0 +1,6 @@ +-- Up + +ALTER TABLE links ADD COLUMN faviconCache TEXT NULL DEFAULT NULL; +UPDATE links SET expiresAt = null WHERE 1=1; + +-- Down diff --git a/server/analyseLinks.js b/server/analyseLinks.js index 8652d6985..008c65aae 100644 --- a/server/analyseLinks.js +++ b/server/analyseLinks.js @@ -3,6 +3,7 @@ import './setup.ts'; import dbConnection from './db.ts'; import SQL from 'sql-template-strings'; import { LinkAnalyser } from '../src/links.js'; +import copyImage from './imageCopy.ts'; const timer = (ms) => new Promise((res) => setTimeout(res, ms)); @@ -17,7 +18,12 @@ const timer = (ms) => new Promise((res) => setTimeout(res, ms)); continue; } const results = await Promise.all(chunk.map(({ url }) => Promise.race([ - analyser.analyse(url), + analyser.analyse(url).then(async (result) => { + if (result.favicon) { + result.faviconCache = await copyImage('favicon-cache', result.favicon, 30); + } + return result; + }), new Promise((resolve) => setTimeout(() => resolve({ url, error: new Error('timeout') }), 12000)), ]))); for (const result of results) { @@ -30,6 +36,7 @@ const timer = (ms) => new Promise((res) => setTimeout(res, ms)); await db.get(SQL`UPDATE links SET expiresAt = ${expireAt}, favicon = ${result.favicon}, + faviconCache = ${result.faviconCache}, relMe = ${JSON.stringify(result.relMe)}, nodeinfo = ${JSON.stringify(result.nodeinfo)} WHERE url=${result.url}`); diff --git a/server/avatarCopy.ts b/server/avatarCopy.ts deleted file mode 100644 index 51ed372be..000000000 --- a/server/avatarCopy.ts +++ /dev/null @@ -1,39 +0,0 @@ -import fetch from 'node-fetch'; -import { S3 } from '@aws-sdk/client-s3'; -import { awsConfig, awsParams } from './aws.ts'; -import md5 from 'js-md5'; -import { loadImage, createCanvas } from 'canvas'; - -const s3 = new S3(awsConfig); - -export default async (provider: string, url: string): Promise => { - if (!url) { - return null; - } - - const key = `images-copy/${provider}/${md5(url)}.png`; - - try { - await s3.headObject({ Key: key, ...awsParams }); - - return `${process.env.CLOUDFRONT}/${key}`; - } catch { - try { - const image = await loadImage(Buffer.from(await (await fetch(url)).arrayBuffer())); - const canvas = createCanvas(image.width, image.height); - canvas.getContext('2d').drawImage(image, 0, 0); - - await s3.putObject({ - Key: key, - Body: canvas.toBuffer('image/png'), - ContentType: 'image/png', - ACL: 'public-read', - ...awsParams, - }); - - return `${process.env.CLOUDFRONT}/${key}`; - } catch (e) { - return null; - } - } -}; diff --git a/server/imageCopy.ts b/server/imageCopy.ts new file mode 100644 index 000000000..607c1af51 --- /dev/null +++ b/server/imageCopy.ts @@ -0,0 +1,55 @@ +import fetch from 'node-fetch'; +import { S3 } from '@aws-sdk/client-s3'; +import { awsConfig, awsParams } from './aws.ts'; +import md5 from 'js-md5'; +import { loadImage, createCanvas } from 'canvas'; + +const s3 = new S3(awsConfig); + +const convertToPng = async (original: Buffer): Promise => { + const image = await loadImage(original); + const canvas = createCanvas(image.width, image.height); + canvas.getContext('2d').drawImage(image, 0, 0); + + return canvas.toBuffer('image/png'); +}; + +export default async (prefix: string, url: string, ttlDays: number | null = null): Promise => { + if (!url) { + return null; + } + + const isSvg = url.toLowerCase().endsWith('.svg'); + + const key = `${prefix}/${md5(url)}.${isSvg ? 'svg' : 'png'}`; + + try { + const metadata = await s3.headObject({ Key: key, ...awsParams }) as any; + + if (ttlDays !== null && metadata!.LastModified < new Date(new Date().setDate(new Date().getDate() - ttlDays))) { + throw 'force refresh'; + } + + return `${process.env.CLOUDFRONT}/${key}`; + } catch { + try { + const originalBuffer = Buffer.from(await (await fetch(url)).arrayBuffer()); + + await s3.putObject({ + Key: key, + Body: isSvg + ? originalBuffer + : await convertToPng(originalBuffer), + ContentType: isSvg + ? 'image/svg+xml' + : 'image/png', + ACL: 'public-read', + ...awsParams, + }); + + return `${process.env.CLOUDFRONT}/${key}`; + } catch (e) { + return null; + } + } +}; diff --git a/server/routes/profile.js b/server/routes/profile.js index b8982aad3..49e721801 100644 --- a/server/routes/profile.js +++ b/server/routes/profile.js @@ -262,7 +262,7 @@ const fetchProfiles = async (db, username, self, opts = undefined) => { const linksMetadata = {}; for (const link of await db.all(SQL`SELECT * FROM links WHERE url IN (`.append(links.map((k) => `'${k.replace(/'/g, '\'\'')}'`).join(',')).append(SQL`)`))) { linksMetadata[link.url] = { - favicon: link.favicon, + favicon: link.faviconCache || link.favicon, relMe: JSON.parse(link.relMe), nodeinfo: JSON.parse(link.nodeinfo), }; diff --git a/server/routes/user.js b/server/routes/user.js index 613c09e2a..b50819856 100644 --- a/server/routes/user.js +++ b/server/routes/user.js @@ -14,7 +14,7 @@ import assert from 'assert'; import { addMfaInfo } from './mfa.js'; import buildLocaleList from '../../src/buildLocaleList.ts'; import { lookupBanArchive } from '../ban.js'; -import copyAvatar from '../avatarCopy.ts'; +import copyImage from '../imageCopy.ts'; import { usernameRegex, usernameUnsafeRegex } from '../../src/username.ts'; const config = loadSuml('config'); import auditLog from '../audit.ts'; @@ -653,7 +653,7 @@ router.get('/user/social/:provider', handleErrorAsync(async (req, res) => { await invalidateAuthenticatorsOfType(req.db, dbUser.id, req.params.provider); if (!payload.avatarCopy && payload.avatar) { - payload.avatarCopy = await copyAvatar(req.params.provider, payload.avatar); + payload.avatarCopy = await copyImage(`images-copy/${req.params.provider}`, payload.avatar); } await saveAuthenticator(req.db, req.params.provider, dbUser, payload); From 67156361b2d37231918aa763dc650b83ae1ddb4b Mon Sep 17 00:00:00 2001 From: Valentyne Stigloher Date: Fri, 14 Jun 2024 19:58:55 +0200 Subject: [PATCH 2/2] (ts) replace any type assertion with more specific null assertion --- server/imageCopy.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/imageCopy.ts b/server/imageCopy.ts index 607c1af51..b7b31a87c 100644 --- a/server/imageCopy.ts +++ b/server/imageCopy.ts @@ -24,9 +24,9 @@ export default async (prefix: string, url: string, ttlDays: number | null = null const key = `${prefix}/${md5(url)}.${isSvg ? 'svg' : 'png'}`; try { - const metadata = await s3.headObject({ Key: key, ...awsParams }) as any; + const metadata = await s3.headObject({ Key: key, ...awsParams }); - if (ttlDays !== null && metadata!.LastModified < new Date(new Date().setDate(new Date().getDate() - ttlDays))) { + if (ttlDays !== null && metadata.LastModified! < new Date(new Date().setDate(new Date().getDate() - ttlDays))) { throw 'force refresh'; }