sparcs-kaist / zabo-server-nodejs

Image and Poster Advertising Service @ KAIST
https://zabo.sparcs.org
9 stars 3 forks source link

[Bug] Server crashes when invalid session id is given #102

Closed SnowSuno closed 1 year ago

SnowSuno commented 1 year ago

Describe the bug

A clear and concise description of what the bug is.

To Reproduce

/api/auth에 특정 토큰을 헤더로 넣어서 요청을 전송합니다.

이때 특정 헤더는 valid한 jwt 토큰이지만, decode한 값은 존재하지 않는 sid이어야 합니다. 제 경우, dev 서버 기준 에러를 발생시키는 토큰은 다음과 같았습니다:

eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJfaWQiOiI2NDI3MDQ3YjNjZWFiZDAwMWJmMGM2MDQiLCJzaWQiOiIwYjEzZjA1ODg5NmVlYjA1YmUwYiIsImVtYWlsIjoic25vd3N1bm9Ac3BhcmNzLm9yZyIsInVzZXJuYW1lIjoi6raM7Iic7Zi4NTMiLCJpYXQiOjE2ODMzNzkzNDMsImV4cCI6MTY4ODU2MzM0MywiaXNzIjoiemFiby1zcGFyY3Mta2Fpc3QifQ.2TF7LVCL9vw8Ph5dyvRen7lGlOg4CSpPZsaDEPdk3q0

다른 아무 string을 dev 서버의 secret key로 encode해도 같은 결과를 얻을 수 있을 것으로 보입니다.

재현을 쉽게 하기 위해 session storage의 'token' 키의 값을 위 토큰으로 입력하고, 홈 화면에서 새로고침하여 요청을 보내면 서버가 502 응답을 반환하면서 죽는 것을 확인할 수 있습니다.

Screenshots

image image
SnowSuno commented 1 year ago

https://github.com/sparcs-kaist/zabo-server-nodejs/blob/feature/share-link/src/controllers/auth.js#L25-L31

const user = await User.findOne({ sso_sid: sid })
  .populate({
    path: "groups",
    select: "name profilePhoto followers recentUpload subtitle",
  })
  .populate("boards");
if (user.isAdmin) {

위 부분에서 해당 sid에 대한 usernull인 경우에 대한 경우가 handle 되지 않아 Cannot read properties of null (reading 'isAdmin') 에러가 발생하는 문제입니다.

SnowSuno commented 1 year ago

Global error handler에서 에러가 잡히지 않고 서버가 죽어버리는 이유는 jwt.verify의 callback 함수로 async 함수를 사용했고, 이에 대한 handling은 이루어져 있지 않아 이 callback 함수 내부에서 발생하는 에러에 대해서는 unhandled promise rejection이 발생하기 때문입니다.

https://github.com/sparcs-kaist/zabo-server-nodejs/blob/feature/share-link/src/controllers/auth.js#L15

jwt.verify(token, jwtSecret, async (error, decoded) => {
withSang commented 1 year ago

에러 발생 상황, 발생 원인에 대한 상세한 설명 주셔서 너무 감사드립니다. 감동이네요 👍