google / jwt_verify_lib

Provide c++ library to verify JWT tokens
Apache License 2.0
42 stars 43 forks source link

JWT with negative expiration is classified as valid #68

Closed A1cor closed 2 years ago

A1cor commented 2 years ago

Hi,

JWT with a very small negative value in "exp" are classifies as valid. Example:

{
  "nbf": 1663663666,
  "exp": -386380800,
  "iss": "issuer",
  "aud": "audience"
}

"iat", "nbf" and "exp" are parsed into a uint64_t type variable by the "GetInt64" function, not verifying they are positive numbers:

if (payload_getter.GetInt64("iat", &iat_) == StructUtils::WRONG_TYPE) {
    return Status::JwtPayloadParseErrorIatNotInteger;
  }
  if (payload_getter.GetInt64("nbf", &nbf_) == StructUtils::WRONG_TYPE) {
    return Status::JwtPayloadParseErrorNbfNotInteger;
  }
  if (payload_getter.GetInt64("exp", &exp_) == StructUtils::WRONG_TYPE) {
    return Status::JwtPayloadParseErrorExpNotInteger;
  }
StructUtils::FindResult StructUtils::GetInt64(const std::string& name,
                                              uint64_t* value) {
  const auto& fields = struct_pb_.fields();
  const auto it = fields.find(name);
  if (it == fields.end()) {
    return MISSING;
  }
  if (it->second.kind_case() != google::protobuf::Value::kNumberValue) {
    return WRONG_TYPE;
  }
  *value = static_cast<uint64_t>(it->second.number_value());
  return OK;
}

In case of a negative number, a large number is stored in "value" and the JWT is classified as not expired:

Status Jwt::verifyTimeConstraint(uint64_t now, uint64_t clock_skew) const {
  // Check Jwt is active (nbf).
  if (now + clock_skew < nbf_) {
    return Status::JwtNotYetValid;
  }
  // Check JWT has not expired (exp).
  if (exp_ && now > exp_ + clock_skew) {
    return Status::JwtExpired;
  }
  return Status::Ok;
}
qiwzhang commented 2 years ago

It is a bug. Thanks for reporting it.