Fix path traversal, auth-bypass, and DoS findings from the security review

- Add ValidateFileName (allowlist regex, rejects .. and separators) and apply
  it to every route that concatenates a raw path segment into a filesystem or
  remote FTP path: /show/lvgmc-forecast, /show/grib, /grib/binary-chunk, and
  /debug/file. Previously an unauthenticated caller could read arbitrary
  files, including /proc/self/environ (leaks LVGMC_PASSWORD/POSTGRES_PASSWORD).
- Harden ValidateInt to reject negative integers.
- Gate /api/fetch/lvgmc/stations behind ENABLE_LVGMC_FTP_JOBS so it can no
  longer trigger a real, unauthenticated FTP login regardless of the flag;
  stop leaking error.getMessage in its response.
- Add an explicit /api/* catch-all (NotFound) so an unmatched API route can
  never fall through to the SPA fallback and be served index.html as a 200.
- Cap binary-chunk read length at 64MB to prevent an unbounded allocation.
This commit is contained in:
b0txec
2026-08-24 11:50:53 +03:00
parent fdd5508e43
commit 6b9c7cf4ae
3 changed files with 50 additions and 16 deletions
+9
View File
@@ -81,7 +81,16 @@ class DataService(log: Logger[IO]) {
Executors.newFixedThreadPool(8) // limit concurrency Executors.newFixedThreadPool(8) // limit concurrency
) )
private val MAX_BINARY_CHUNK_BYTES = 64 * 1024 * 1024
def getBinaryChunk(offset: Int, length: Int, fileName: String): IO[Array[Byte]] = { def getBinaryChunk(offset: Int, length: Int, fileName: String): IO[Array[Byte]] = {
if (length > MAX_BINARY_CHUNK_BYTES || length < 0)
IO.raiseError(new IllegalArgumentException(s"Requested length $length is invalid (max $MAX_BINARY_CHUNK_BYTES bytes)"))
else
getBinaryChunkUnchecked(offset, length, fileName)
}
private def getBinaryChunkUnchecked(offset: Int, length: Int, fileName: String): IO[Array[Byte]] = {
val fileResource = Resource.make( val fileResource = Resource.make(
IO.blocking(new RandomAccessFile(s"$GRIB_FOLDER/$fileName", "r")) IO.blocking(new RandomAccessFile(s"$GRIB_FOLDER/$fileName", "r"))
)(file => IO.blocking(file.close())) )(file => IO.blocking(file.close()))
+19 -6
View File
@@ -10,7 +10,7 @@ import fetch.csv.FileNameService
import fetch.lvgmc.{FetchService, WaterTemperatureService} import fetch.lvgmc.{FetchService, WaterTemperatureService}
import fetch.warnings.WarningService import fetch.warnings.WarningService
import fs2.io.file.{Files, Path} import fs2.io.file.{Files, Path}
import server.ValidateRoutes.{AggFieldList, AggKey, CityList, DateTimeRange, Granularity, ValidateDate, ValidateDateTime, ValidateInt, ValidateMonths, ValidateZonedDateTime} import server.ValidateRoutes.{AggFieldList, AggKey, CityList, DateTimeRange, Granularity, ValidateDate, ValidateDateTime, ValidateFileName, ValidateInt, ValidateMonths, ValidateZonedDateTime}
import org.http4s._ import org.http4s._
import org.http4s.dsl.io._ import org.http4s.dsl.io._
import org.http4s.implicits._ import org.http4s.implicits._
@@ -64,13 +64,17 @@ class Server(postgresService: PostgresService, dataService: DataService, fetch:
} }
// http://0.0.0.0:8080/api/show/lvgmc-forecast/Latvija_LTV_pilsetas_tekosa_dn.csv // http://0.0.0.0:8080/api/show/lvgmc-forecast/Latvija_LTV_pilsetas_tekosa_dn.csv
case GET -> Root / "show" / "lvgmc-forecast" / fileName => case GET -> Root / "show" / "lvgmc-forecast" / ValidateFileName(fileName) =>
fetch.fetchFile(fileName).flatMap(bytes => fetch.fetchFile(fileName).flatMap(bytes =>
Ok(bytes).map(_.withContentType(`Content-Type`(MediaType.text.csv))) Ok(bytes).map(_.withContentType(`Content-Type`(MediaType.text.csv)))
) )
// http://0.0.0.0:8080/api/fetch/lvgmc/stations // http://0.0.0.0:8080/api/fetch/lvgmc/stations
// Gated behind the same flag as the scheduled FTP task: this route would
// otherwise let anyone unauthenticated trigger a real LVGMC FTP login on
// demand, bypassing ENABLE_LVGMC_FTP_JOBS entirely.
case GET -> Root / "fetch" / "lvgmc" / "stations" => case GET -> Root / "fetch" / "lvgmc" / "stations" =>
if (sys.env.get("ENABLE_LVGMC_FTP_JOBS").exists(_.equalsIgnoreCase("true")))
( (
for { for {
fileName <- new FileNameService().generateCurrentHour fileName <- new FileNameService().generateCurrentHour
@@ -80,8 +84,11 @@ class Server(postgresService: PostgresService, dataService: DataService, fetch:
) )
.flatMap(content => Ok(content)) .flatMap(content => Ok(content))
.handleErrorWith(error => .handleErrorWith(error =>
InternalServerError(s"Failed to fetch stations: ${error.getMessage}") log.error(error)("Failed to fetch LVGMC stations") *>
InternalServerError("Failed to fetch stations")
) )
else
ServiceUnavailable("LVGMC FTP fetching is currently disabled")
// http://0.0.0.0:8080/api/show/grib-all-structure // http://0.0.0.0:8080/api/show/grib-all-structure
case GET -> Root / "show" / "grib-all-structure" => case GET -> Root / "show" / "grib-all-structure" =>
@@ -92,10 +99,10 @@ class Server(postgresService: PostgresService, dataService: DataService, fetch:
dataService.getFileList().flatMap(fileList => Ok(fileList.asJson)) dataService.getFileList().flatMap(fileList => Ok(fileList.asJson))
// http://0.0.0.0:8080/api/show/grib-name/harmonie_2025-02-01T1500Z_2025-02-01T180000Z.grib // http://0.0.0.0:8080/api/show/grib-name/harmonie_2025-02-01T1500Z_2025-02-01T180000Z.grib
case GET -> Root / "show" / "grib" / fileName => case GET -> Root / "show" / "grib" / ValidateFileName(fileName) =>
dataService.getGribStucture(fileName).flatMap(response => Ok(response.asJson)) dataService.getGribStucture(fileName).flatMap(response => Ok(response.asJson))
case GET -> Root / "grib" / "binary-chunk" / ValidateInt(binaryOffset) / ValidateInt(binaryLength) / fileName => case GET -> Root / "grib" / "binary-chunk" / ValidateInt(binaryOffset) / ValidateInt(binaryLength) / ValidateFileName(fileName) =>
dataService.getBinaryChunk(binaryOffset, binaryLength, fileName).flatMap(buffer => Ok(buffer)) dataService.getBinaryChunk(binaryOffset, binaryLength, fileName).flatMap(buffer => Ok(buffer))
// http://0.0.0.0:8080/api/grib/delete-old-forecasts // http://0.0.0.0:8080/api/grib/delete-old-forecasts
@@ -117,7 +124,7 @@ class Server(postgresService: PostgresService, dataService: DataService, fetch:
DebugUtils.getFolderStructure.flatMap(response => Ok(response.asJson)) DebugUtils.getFolderStructure.flatMap(response => Ok(response.asJson))
// http://0.0.0.0:8080/api/debug/file/error_2025-03-16_152201.txt // http://0.0.0.0:8080/api/debug/file/error_2025-03-16_152201.txt
case GET -> Root / "debug" / "file" / fileName => case GET -> Root / "debug" / "file" / ValidateFileName(fileName) =>
val filePath = Path(s"data/tmp/$fileName") val filePath = Path(s"data/tmp/$fileName")
Files[IO].exists(filePath).flatMap { Files[IO].exists(filePath).flatMap {
@@ -193,6 +200,12 @@ class Server(postgresService: PostgresService, dataService: DataService, fetch:
// http://0.0.0.0:8080/api/show/datetime/20230423_1300 // http://0.0.0.0:8080/api/show/datetime/20230423_1300
case GET -> Root / "show" / "datetime" / ValidateDateTime(datetime) => case GET -> Root / "show" / "datetime" / ValidateDateTime(datetime) =>
postgresService.getDateTimeEntries(datetime).flatMap(content => Ok(content.asJson)) postgresService.getDateTimeEntries(datetime).flatMap(content => Ok(content.asJson))
// Explicit catch-all: guarantees every /api/* request resolves inside
// this route set (never falls through to the SPA fallback below and
// gets served index.html as a false 200).
case _ =>
NotFound()
} }
private val corsConfig = CORSConfig.default private val corsConfig = CORSConfig.default
+13 -1
View File
@@ -92,7 +92,19 @@ object ValidateRoutes {
object ValidateInt { object ValidateInt {
def unapply(str: String): Option[Int] = { def unapply(str: String): Option[Int] = {
Option(str.toInt) Try(str.toInt).toOption.filter(_ >= 0)
}
}
// Rejects path traversal and directory separators (including URL-decoded
// %2f, which arrives here as a literal '/' after http4s decodes the path
// segment) by allowlisting a safe filename character set, rather than
// trying to blocklist every encoding of "..". Used anywhere a path
// segment gets concatenated directly into a filesystem Path.
object ValidateFileName {
private val safeFileName = "^[A-Za-z0-9._-]+$".r
def unapply(str: String): Option[String] = {
if (str.nonEmpty && !str.contains("..") && safeFileName.matches(str)) Some(str) else None
} }
} }
} }