twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 405 forks source link

Update sbt to 1.6.1, which upgrades the log4j dependency to 2.17.1 #574

Closed ggrossman closed 2 years ago

ggrossman commented 2 years ago

Problem

sbt has a dependency on log4j2, which was shown recently to have serious security vulnerabilities. The version of sbt currently used by finatra, 1.5.5, is vulnerable.

Solution

Update to use sbt 1.6.1, which upgrades the log4j dependency to log4j 2.17.1, which resolves these security vulnerabilities.

From https://eed3si9n.com/sbt-1.6.1:

sbt 1.6.1 updates log4j 2 to 2.17.1, which fixes a remote code execution vulnerability when attacker controls configuration (CVE-2021-44832) #6765 by @eed3si9n

For details, see The state of the log4j CVE in the Scala ecosystem

codecov[bot] commented 2 years ago

Codecov Report

Merging #574 (dfb5a96) into develop (10e1fc6) will decrease coverage by 0.05%. The diff coverage is n/a.

:exclamation: Current head dfb5a96 differs from pull request most recent head c3be386. Consider uploading reports for the commit c3be386 to get more accurate results Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #574      +/-   ##
===========================================
- Coverage    89.93%   89.87%   -0.06%     
===========================================
  Files          223      227       +4     
  Lines         3626     3666      +40     
  Branches       202      209       +7     
===========================================
+ Hits          3261     3295      +34     
- Misses         365      371       +6     
Impacted Files Coverage Δ
...nject/thrift/modules/ThriftClientModuleTrait.scala 50.00% <0.00%> (-7.15%) :arrow_down:
...tter/finatra/http/filters/HttpResponseFilter.scala 93.33% <0.00%> (-4.29%) :arrow_down:
...com/twitter/finatra/http/filters/StatsFilter.scala 86.95% <0.00%> (-3.75%) :arrow_down:
...a/com/twitter/finatra/thrift/routing/routers.scala 88.46% <0.00%> (-1.87%) :arrow_down:
.../finatra/http/marshalling/MessageBodyManager.scala 93.02% <0.00%> (-1.03%) :arrow_down:
...tra/http/marshalling/MessageInjectableValues.scala 90.40% <0.00%> (-0.59%) :arrow_down:
...main/scala/com/twitter/inject/server/Awaiter.scala 100.00% <0.00%> (ø)
...ain/scala/com/twitter/inject/conversions/map.scala 100.00% <0.00%> (ø)
...om/twitter/finatra/http/request/RequestUtils.scala 100.00% <0.00%> (ø)
...in/scala/com/twitter/inject/server/PortUtils.scala 100.00% <0.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 10e1fc6...c3be386. Read the comment docs.

mosesn commented 2 years ago

@ggrossman looks like our OSS CI job is failing, so it's hard to see whether this patch is healthy or not. Give us a little time and we'll get things back up and running.

mosesn commented 2 years ago

Thanks, we ended up testing it on our own infrastructure and it looks good. Will fix the bad test shortly. This was merged in here!