From 714b8640dac9db46bcd53ddd99641fad917b914c Mon Sep 17 00:00:00 2001 From: emeric Date: Sun, 12 Jul 2026 12:38:35 +0200 Subject: [PATCH] Validate podcast URLs and probe artwork before saving --- src/libs/core/CMakeLists.txt | 1 + src/libs/core/impl/http/SendQueue.cpp | 7 ++ src/libs/core/impl/http/UrlValidation.cpp | 28 +++++++ .../core/include/core/http/UrlValidation.hpp | 28 +++++++ src/libs/core/test/CMakeLists.txt | 1 + src/libs/core/test/HttpUrlValidation.cpp | 43 ++++++++++ src/libs/services/podcast/CMakeLists.txt | 1 + .../services/podcast/impl/UrlValidation.cpp | 30 +++++++ .../services/podcast/impl/UrlValidation.hpp | 27 ++++++ .../steps/DownloadEpisodeArtworksStep.cpp | 19 ++++- .../impl/steps/DownloadEpisodesStep.cpp | 2 + .../steps/DownloadPodcastArtworksStep.cpp | 19 ++++- .../impl/steps/RefreshPodcastsStep.cpp | 82 +++++++++++++------ 13 files changed, 259 insertions(+), 29 deletions(-) create mode 100644 src/libs/core/impl/http/UrlValidation.cpp create mode 100644 src/libs/core/include/core/http/UrlValidation.hpp create mode 100644 src/libs/core/test/HttpUrlValidation.cpp create mode 100644 src/libs/services/podcast/impl/UrlValidation.cpp create mode 100644 src/libs/services/podcast/impl/UrlValidation.hpp diff --git a/src/libs/core/CMakeLists.txt b/src/libs/core/CMakeLists.txt index 65c8826d..5b089608 100644 --- a/src/libs/core/CMakeLists.txt +++ b/src/libs/core/CMakeLists.txt @@ -13,6 +13,7 @@ configure_file( add_library(lmscore STATIC impl/http/Client.cpp impl/http/SendQueue.cpp + impl/http/UrlValidation.cpp impl/media/Codec.cpp impl/media/Container.cpp impl/media/ImageType.cpp diff --git a/src/libs/core/impl/http/SendQueue.cpp b/src/libs/core/impl/http/SendQueue.cpp index e4c819ec..33b2eac3 100644 --- a/src/libs/core/impl/http/SendQueue.cpp +++ b/src/libs/core/impl/http/SendQueue.cpp @@ -31,6 +31,7 @@ #include "core/ILogger.hpp" #include "core/ITraceLogger.hpp" #include "core/String.hpp" +#include "core/http/UrlValidation.hpp" #define LOG(sev, message) LMS_LOG(HTTP, sev, "[Http SendQueue] - " << message) @@ -190,6 +191,12 @@ namespace lms::core::http const std::string url{ _baseUrl + request.getParameters().relativeUrl }; LOG(DEBUG, "Sending " << (request.getType() == ClientRequest::Type::GET ? "GET" : "POST") << " request to url '" << url << "'"); + if (!isValidUrl(url)) + { + LOG(ERROR, "Refusing request to '" << url << "': invalid URL"); + return false; + } + _client.setMaximumResponseSize(request.getParameters().onChunkReceived ? 0 : request.getParameters().responseBufferSize); bool res{}; diff --git a/src/libs/core/impl/http/UrlValidation.cpp b/src/libs/core/impl/http/UrlValidation.cpp new file mode 100644 index 00000000..d6269785 --- /dev/null +++ b/src/libs/core/impl/http/UrlValidation.cpp @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include "core/http/UrlValidation.hpp" + +namespace lms::core::http +{ + bool isValidUrl(std::string_view url) + { + return !url.empty() && (url.starts_with("http://") || url.starts_with("https://")); + } +} // namespace lms::core::http diff --git a/src/libs/core/include/core/http/UrlValidation.hpp b/src/libs/core/include/core/http/UrlValidation.hpp new file mode 100644 index 00000000..61dadee1 --- /dev/null +++ b/src/libs/core/include/core/http/UrlValidation.hpp @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#pragma once + +#include + +namespace lms::core::http +{ + // Returns true if url is a well-formed absolute URL with an http:// or https:// scheme. + bool isValidUrl(std::string_view url); +} // namespace lms::core::http diff --git a/src/libs/core/test/CMakeLists.txt b/src/libs/core/test/CMakeLists.txt index 074abeb0..73ce60d2 100644 --- a/src/libs/core/test/CMakeLists.txt +++ b/src/libs/core/test/CMakeLists.txt @@ -2,6 +2,7 @@ include(GoogleTest) add_executable(test-core EnumSet.cpp + HttpUrlValidation.cpp JobScheduler.cpp LiteralString.cpp PartialDateTime.cpp diff --git a/src/libs/core/test/HttpUrlValidation.cpp b/src/libs/core/test/HttpUrlValidation.cpp new file mode 100644 index 00000000..aa24df99 --- /dev/null +++ b/src/libs/core/test/HttpUrlValidation.cpp @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include + +#include "core/http/UrlValidation.hpp" + +namespace lms::core::http::tests +{ + TEST(HttpUrlValidation, AllowedUrls) + { + EXPECT_TRUE(isValidUrl("http://feeds.example.com/podcast.rss")); + EXPECT_TRUE(isValidUrl("https://feeds.example.com/podcast.rss")); + EXPECT_TRUE(isValidUrl("http://192.168.1.100/feed.rss")); + EXPECT_TRUE(isValidUrl("https://example.com/episode.mp3")); + } + + TEST(HttpUrlValidation, DisallowedUrls) + { + EXPECT_FALSE(isValidUrl("")); + EXPECT_FALSE(isValidUrl("ftp://attacker.com/feed")); + EXPECT_FALSE(isValidUrl("file:///etc/passwd")); + EXPECT_FALSE(isValidUrl("javascript:alert(1)")); + EXPECT_FALSE(isValidUrl("//example.com/feed")); + EXPECT_FALSE(isValidUrl("HTTP://example.com/feed")); // scheme check is case-sensitive + } +} // namespace lms::core::http::tests diff --git a/src/libs/services/podcast/CMakeLists.txt b/src/libs/services/podcast/CMakeLists.txt index 1921939b..18be7584 100644 --- a/src/libs/services/podcast/CMakeLists.txt +++ b/src/libs/services/podcast/CMakeLists.txt @@ -11,6 +11,7 @@ add_library(lmspodcast STATIC impl/Executor.cpp impl/PodcastParsing.cpp impl/PodcastService.cpp + impl/UrlValidation.cpp ) target_include_directories(lmspodcast INTERFACE diff --git a/src/libs/services/podcast/impl/UrlValidation.cpp b/src/libs/services/podcast/impl/UrlValidation.cpp new file mode 100644 index 00000000..2ef78cf3 --- /dev/null +++ b/src/libs/services/podcast/impl/UrlValidation.cpp @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#include "UrlValidation.hpp" + +#include "core/http/UrlValidation.hpp" + +namespace lms::podcast +{ + bool isAllowedPodcastUrl(std::string_view url) + { + return core::http::isValidUrl(url); + } +} // namespace lms::podcast diff --git a/src/libs/services/podcast/impl/UrlValidation.hpp b/src/libs/services/podcast/impl/UrlValidation.hpp new file mode 100644 index 00000000..40f45122 --- /dev/null +++ b/src/libs/services/podcast/impl/UrlValidation.hpp @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2025 Emeric Poupon + * + * This file is part of LMS. + * + * LMS is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * LMS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with LMS. If not, see . + */ + +#pragma once + +#include + +namespace lms::podcast +{ + bool isAllowedPodcastUrl(std::string_view url); +} // namespace lms::podcast diff --git a/src/libs/services/podcast/impl/steps/DownloadEpisodeArtworksStep.cpp b/src/libs/services/podcast/impl/steps/DownloadEpisodeArtworksStep.cpp index 1cc6bd6a..6bb06531 100644 --- a/src/libs/services/podcast/impl/steps/DownloadEpisodeArtworksStep.cpp +++ b/src/libs/services/podcast/impl/steps/DownloadEpisodeArtworksStep.cpp @@ -25,11 +25,14 @@ #include "core/ILogger.hpp" #include "core/http/IClient.hpp" + #include "database/IDb.hpp" #include "database/Session.hpp" #include "database/objects/Artwork.hpp" #include "database/objects/Image.hpp" #include "database/objects/PodcastEpisode.hpp" +#include "image/Exception.hpp" +#include "image/Image.hpp" #include "Executor.hpp" #include "Utils.hpp" @@ -122,6 +125,19 @@ namespace lms::podcast }; params.onSuccessFunc = [=, this](const Wt::Http::Message& msg) { getExecutor().post([=, this] { + const std::string body{ msg.body() }; // API enforces a copy here :( + const auto bodySpan{ std::as_bytes(std::span{ body.data(), body.size() }) }; + try + { + image::probeImage(bodySpan); + } + catch (const image::Exception& e) + { + LMS_LOG(PODCAST, WARNING, "Discarding non-image response for episode '" << episode->getTitle() << "' from '" << url << "': " << e.what()); + processNext(); + return; + } + std::ofstream file{ finalFilePath, std::ios::binary | std::ios::trunc }; if (!file) { @@ -131,8 +147,7 @@ namespace lms::podcast return; } - const std::string body{ msg.body() }; // API enforces a copy here - file.write(body.data(), body.size()); + file.write(body.data(), static_cast(body.size())); if (!file) { std::error_code ec{ errno, std::generic_category() }; diff --git a/src/libs/services/podcast/impl/steps/DownloadEpisodesStep.cpp b/src/libs/services/podcast/impl/steps/DownloadEpisodesStep.cpp index a3704984..0953f069 100644 --- a/src/libs/services/podcast/impl/steps/DownloadEpisodesStep.cpp +++ b/src/libs/services/podcast/impl/steps/DownloadEpisodesStep.cpp @@ -212,11 +212,13 @@ namespace lms::podcast else { LMS_LOG(PODCAST, WARNING, "Failed to get audio properties from downloaded episode from '" << url << "'"); + utils::removeFile(tmpFilePath); } } catch (const audio::Exception& e) { LMS_LOG(PODCAST, WARNING, "Failed to parse downloaded episode from '" << url << "': " << e.what()); + utils::removeFile(tmpFilePath); } processNext(); diff --git a/src/libs/services/podcast/impl/steps/DownloadPodcastArtworksStep.cpp b/src/libs/services/podcast/impl/steps/DownloadPodcastArtworksStep.cpp index 2db0e696..352bccdb 100644 --- a/src/libs/services/podcast/impl/steps/DownloadPodcastArtworksStep.cpp +++ b/src/libs/services/podcast/impl/steps/DownloadPodcastArtworksStep.cpp @@ -29,6 +29,8 @@ #include "database/Session.hpp" #include "database/objects/Artwork.hpp" #include "database/objects/Podcast.hpp" +#include "image/Exception.hpp" +#include "image/Image.hpp" #include "Executor.hpp" #include "Utils.hpp" @@ -121,9 +123,20 @@ namespace lms::podcast }; params.onSuccessFunc = [=, this](const Wt::Http::Message& msg) { getExecutor().post([=, this] { - const std::string body{ msg.body() }; // API enforces a copy here + const std::string body{ msg.body() }; // API enforces a copy here :( + const auto bodySpan{ std::as_bytes(std::span{ body.data(), body.size() }) }; + try + { + image::probeImage(bodySpan); + } + catch (const image::Exception& e) + { + LMS_LOG(PODCAST, WARNING, "Discarding non-image response for podcast artwork from '" << url << "': " << e.what()); + processNext(); + return; + } - std::ofstream file{ finalFilePath, std::ios::binary | std::ios::app }; + std::ofstream file{ finalFilePath, std::ios::binary | std::ios::trunc }; if (!file) { std::error_code ec{ errno, std::generic_category() }; @@ -132,7 +145,7 @@ namespace lms::podcast return; } - file.write(body.data(), body.size()); + file.write(body.data(), static_cast(body.size())); if (!file) { std::error_code ec{ errno, std::generic_category() }; diff --git a/src/libs/services/podcast/impl/steps/RefreshPodcastsStep.cpp b/src/libs/services/podcast/impl/steps/RefreshPodcastsStep.cpp index d43561ce..0ac7233f 100644 --- a/src/libs/services/podcast/impl/steps/RefreshPodcastsStep.cpp +++ b/src/libs/services/podcast/impl/steps/RefreshPodcastsStep.cpp @@ -31,6 +31,7 @@ #include "Executor.hpp" #include "PodcastParsing.hpp" #include "PodcastTypes.hpp" +#include "UrlValidation.hpp" namespace lms::podcast { @@ -49,6 +50,40 @@ namespace lms::podcast image.remove(); } + void addEpisode(db::Session& session, const db::Podcast::pointer& dbPodcast, const PodcastEpisode& episode) + { + LMS_LOG(PODCAST, DEBUG, "Adding episode '" << episode.title << "' to podcast '" << dbPodcast->getTitle() << "'"); + + auto dbEpisode{ session.create(dbPodcast) }; + + dbEpisode.modify()->setAuthor(episode.author); + dbEpisode.modify()->setCategory(episode.category); + dbEpisode.modify()->setDescription(episode.description); + + if (isAllowedPodcastUrl(episode.enclosureUrl.url)) + { + dbEpisode.modify()->setEnclosureUrl(episode.enclosureUrl.url); + dbEpisode.modify()->setEnclosureContentType(episode.enclosureUrl.type); + dbEpisode.modify()->setEnclosureLength(episode.enclosureUrl.length); + } + else + { + LMS_LOG(PODCAST, WARNING, "Episode '" << episode.title << "' : ignoring enclosure URL '" << episode.enclosureUrl.url << "' (bad URL)"); + } + + dbEpisode.modify()->setExplicit(episode.explicitContent ? *episode.explicitContent : false); + dbEpisode.modify()->setLink(episode.link); + dbEpisode.modify()->setPubDate(episode.pubDate); + dbEpisode.modify()->setTitle(episode.title); + + if (isAllowedPodcastUrl(episode.imageUrl)) + dbEpisode.modify()->setImageUrl(episode.imageUrl); + else if (!episode.imageUrl.empty()) + LMS_LOG(PODCAST, WARNING, "Episode '" << episode.title << "' : ignoring image URL '" << episode.imageUrl << "' (bad URL)"); + + dbEpisode.modify()->setDuration(episode.duration); + } + void updatePodcast(db::Session& session, db::PodcastId podcastId, const Podcast& podcast) { auto transaction{ session.createWriteTransaction() }; @@ -62,8 +97,15 @@ namespace lms::podcast // force update the podcast data if (!podcast.newUrl.empty() && podcast.newUrl != dbPodcast->getUrl()) { - LMS_LOG(PODCAST, INFO, "Podcast '" << podcast.title << "' : URL changed from '" << dbPodcast->getUrl() << "' to '" << podcast.newUrl << "'"); - dbPodcast.modify()->setUrl(podcast.newUrl); + if (isAllowedPodcastUrl(podcast.newUrl)) + { + LMS_LOG(PODCAST, INFO, "Podcast '" << podcast.title << "' : URL changed from '" << dbPodcast->getUrl() << "' to '" << podcast.newUrl << "'"); + dbPodcast.modify()->setUrl(podcast.newUrl); + } + else + { + LMS_LOG(PODCAST, WARNING, "Podcast '" << podcast.title << "' : ignoring new podcast URL '" << podcast.newUrl << "' (bad URL)"); + } } dbPodcast.modify()->setAuthor(podcast.author); dbPodcast.modify()->setCategory(podcast.category); @@ -80,14 +122,21 @@ namespace lms::podcast dbPodcast.modify()->setTitle(podcast.title); if (std::string previousUrl{ dbPodcast->getImageUrl() }; !previousUrl.empty() && previousUrl != podcast.imageUrl) { - LMS_LOG(PODCAST, INFO, "Podcast '" << podcast.title << "' : image url changed from '" << previousUrl << "' to '" << podcast.imageUrl << "'"); - if (db::Artwork::pointer currentArtwork{ dbPodcast->getArtwork() }) + if (isAllowedPodcastUrl(podcast.imageUrl)) { - removeArtwork(currentArtwork); - dbPodcast.modify()->setArtwork({}); - } + LMS_LOG(PODCAST, INFO, "Podcast '" << podcast.title << "' : image url changed from '" << previousUrl << "' to '" << podcast.imageUrl << "'"); + if (db::Artwork::pointer currentArtwork{ dbPodcast->getArtwork() }) + { + removeArtwork(currentArtwork); + dbPodcast.modify()->setArtwork({}); + } - dbPodcast.modify()->setImageUrl(podcast.imageUrl); + dbPodcast.modify()->setImageUrl(podcast.imageUrl); + } + else + { + LMS_LOG(PODCAST, WARNING, "Podcast '" << podcast.title << "' : ignoring image URL '" << podcast.imageUrl << "' (disallowed scheme)"); + } } // Only create episodes if they are new, do not modify/update existing entries for now @@ -102,22 +151,7 @@ namespace lms::podcast if (previousNewestEpisodeDateTime.isValid() && episode.pubDate <= previousNewestEpisodeDateTime) continue; // consider already in db - LMS_LOG(PODCAST, DEBUG, "Adding episode '" << episode.title << "' to podcast '" << podcast.title << "'"); - - auto dbEpisode{ session.create(dbPodcast) }; - - dbEpisode.modify()->setAuthor(episode.author); - dbEpisode.modify()->setCategory(episode.category); - dbEpisode.modify()->setDescription(episode.description); - dbEpisode.modify()->setEnclosureUrl(episode.enclosureUrl.url); - dbEpisode.modify()->setEnclosureContentType(episode.enclosureUrl.type); - dbEpisode.modify()->setEnclosureLength(episode.enclosureUrl.length); - dbEpisode.modify()->setExplicit(episode.explicitContent ? *episode.explicitContent : false); - dbEpisode.modify()->setLink(episode.link); - dbEpisode.modify()->setPubDate(episode.pubDate); - dbEpisode.modify()->setTitle(episode.title); - dbEpisode.modify()->setImageUrl(episode.imageUrl); - dbEpisode.modify()->setDuration(episode.duration); + addEpisode(session, dbPodcast, episode); } } } // namespace