From ffb80efe2103b9a368ba03a856d809151c41d53b Mon Sep 17 00:00:00 2001
From: MeiMei <30769358+mei23@users.noreply.github.com>
Date: Sun, 16 Dec 2018 01:44:59 +0900
Subject: [PATCH] Return 404 for invalid Object ID (#3627)

* Update activitypub.ts

* Update activitypub.ts

* Update featured.ts

* Update followers.ts

* Update following.ts

* Update outbox.ts

* Fix following, outbox
---
 src/server/activitypub.ts           | 30 ++++++++++++++++++++++++-----
 src/server/activitypub/featured.ts  | 11 ++++++++---
 src/server/activitypub/followers.ts |  9 +++++++--
 src/server/activitypub/following.ts | 12 +++++++++---
 src/server/activitypub/outbox.ts    | 12 +++++++++---
 5 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/src/server/activitypub.ts b/src/server/activitypub.ts
index 0d4e244856..a308c6aaea 100644
--- a/src/server/activitypub.ts
+++ b/src/server/activitypub.ts
@@ -1,4 +1,4 @@
-import * as mongo from 'mongodb';
+import { ObjectID } from 'mongodb';
 import * as Router from 'koa-router';
 const json = require('koa-json-body');
 const httpSignature = require('http-signature');
@@ -64,8 +64,13 @@ router.post('/users/:user/inbox', json(), inbox);
 router.get('/notes/:note', async (ctx, next) => {
 	if (!isActivityPubReq(ctx)) return await next();
 
+	if (!ObjectID.isValid(ctx.params.note)) {
+		ctx.status = 404;
+		return;
+	}
+
 	const note = await Note.findOne({
-		_id: new mongo.ObjectID(ctx.params.note),
+		_id: new ObjectID(ctx.params.note),
 		visibility: { $in: ['public', 'home'] },
 		localOnly: { $ne: true }
 	});
@@ -82,8 +87,13 @@ router.get('/notes/:note', async (ctx, next) => {
 
 // note activity
 router.get('/notes/:note/activity', async ctx => {
+	if (!ObjectID.isValid(ctx.params.note)) {
+		ctx.status = 404;
+		return;
+	}
+
 	const note = await Note.findOne({
-		_id: new mongo.ObjectID(ctx.params.note),
+		_id: new ObjectID(ctx.params.note),
 		visibility: { $in: ['public', 'home'] },
 		localOnly: { $ne: true }
 	});
@@ -112,7 +122,12 @@ router.get('/users/:user/collections/featured', Featured);
 
 // publickey
 router.get('/users/:user/publickey', async ctx => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	const user = await User.findOne({
 		_id: userId,
@@ -146,7 +161,12 @@ async function userInfo(ctx: Router.IRouterContext, user: IUser) {
 }
 
 router.get('/users/:user', async ctx => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	const user = await User.findOne({
 		_id: userId,
diff --git a/src/server/activitypub/featured.ts b/src/server/activitypub/featured.ts
index f400cc416f..12613b3ecf 100644
--- a/src/server/activitypub/featured.ts
+++ b/src/server/activitypub/featured.ts
@@ -1,4 +1,4 @@
-import * as mongo from 'mongodb';
+import { ObjectID } from 'mongodb';
 import * as Router from 'koa-router';
 import config from '../../config';
 import User from '../../models/user';
@@ -9,7 +9,12 @@ import Note from '../../models/note';
 import renderNote from '../../remote/activitypub/renderer/note';
 
 export default async (ctx: Router.IRouterContext) => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	// Verify user
 	const user = await User.findOne({
@@ -24,7 +29,7 @@ export default async (ctx: Router.IRouterContext) => {
 
 	const pinnedNoteIds = user.pinnedNoteIds || [];
 
-	const pinnedNotes = await Promise.all(pinnedNoteIds.map(id => Note.findOne({ _id: id })));
+	const pinnedNotes = await Promise.all(pinnedNoteIds.filter(ObjectID.isValid).map(id => Note.findOne({ _id: id })));
 
 	const renderedNotes = await Promise.all(pinnedNotes.map(note => renderNote(note)));
 
diff --git a/src/server/activitypub/followers.ts b/src/server/activitypub/followers.ts
index 5c809424cc..9c28c98cd8 100644
--- a/src/server/activitypub/followers.ts
+++ b/src/server/activitypub/followers.ts
@@ -1,4 +1,4 @@
-import * as mongo from 'mongodb';
+import { ObjectID } from 'mongodb';
 import * as Router from 'koa-router';
 import config from '../../config';
 import $ from 'cafy'; import ID, { transform } from '../../misc/cafy-id';
@@ -11,7 +11,12 @@ import renderFollowUser from '../../remote/activitypub/renderer/follow-user';
 import { setResponseType } from '../activitypub';
 
 export default async (ctx: Router.IRouterContext) => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	// Get 'cursor' parameter
 	const [cursor = null, cursorErr] = $.type(ID).optional.get(ctx.request.query.cursor);
diff --git a/src/server/activitypub/following.ts b/src/server/activitypub/following.ts
index a46bb9c7ff..97245245ad 100644
--- a/src/server/activitypub/following.ts
+++ b/src/server/activitypub/following.ts
@@ -1,7 +1,8 @@
-import * as mongo from 'mongodb';
+import { ObjectID } from 'mongodb';
 import * as Router from 'koa-router';
 import config from '../../config';
-import $ from 'cafy'; import ID, { transform } from '../../misc/cafy-id';
+import $ from 'cafy';
+import ID, { transform } from '../../misc/cafy-id';
 import User from '../../models/user';
 import Following from '../../models/following';
 import pack from '../../remote/activitypub/renderer';
@@ -11,7 +12,12 @@ import renderFollowUser from '../../remote/activitypub/renderer/follow-user';
 import { setResponseType } from '../activitypub';
 
 export default async (ctx: Router.IRouterContext) => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	// Get 'cursor' parameter
 	const [cursor = null, cursorErr] = $.type(ID).optional.get(ctx.request.query.cursor);
diff --git a/src/server/activitypub/outbox.ts b/src/server/activitypub/outbox.ts
index 6b917ef843..c35298e3a8 100644
--- a/src/server/activitypub/outbox.ts
+++ b/src/server/activitypub/outbox.ts
@@ -1,7 +1,8 @@
-import * as mongo from 'mongodb';
+import { ObjectID } from 'mongodb';
 import * as Router from 'koa-router';
 import config from '../../config';
-import $ from 'cafy'; import ID, { transform } from '../../misc/cafy-id';
+import $ from 'cafy';
+import ID, { transform } from '../../misc/cafy-id';
 import User from '../../models/user';
 import pack from '../../remote/activitypub/renderer';
 import renderOrderedCollection from '../../remote/activitypub/renderer/ordered-collection';
@@ -15,7 +16,12 @@ import renderAnnounce from '../../remote/activitypub/renderer/announce';
 import { countIf } from '../../prelude/array';
 
 export default async (ctx: Router.IRouterContext) => {
-	const userId = new mongo.ObjectID(ctx.params.user);
+	if (!ObjectID.isValid(ctx.params.user)) {
+		ctx.status = 404;
+		return;
+	}
+
+	const userId = new ObjectID(ctx.params.user);
 
 	// Get 'sinceId' parameter
 	const [sinceId, sinceIdErr] = $.type(ID).optional.get(ctx.request.query.since_id);