Skip to content

Commit 03e22a2

Browse files
Refactor PNG validation and add unit tests for process
Moved PNG signature validation logic from confighttp.cpp to process.cpp as check_valid_png, and updated getCover to use validate_app_image_path for image path resolution and validation. Added extensive unit tests for check_valid_png and validate_app_image_path in test_process.cpp. Defined DEFAULT_APP_IMAGE_PATH in process.h and improved error handling and logging for invalid or unreadable PNG files.
1 parent 474354d commit 03e22a2

4 files changed

Lines changed: 340 additions & 69 deletions

File tree

src/confighttp.cpp

Lines changed: 14 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -284,39 +284,6 @@ namespace confighttp {
284284
return true;
285285
}
286286

287-
/**
288-
* @brief Validates a path whether it is a valid png.
289-
* @param path The path to the png file.
290-
*/
291-
bool check_valid_png(fs::path path) {
292-
// PNG signature as defined in PNG specification
293-
// http://www.libpng.org/pub/png/spec/1.2/PNG-Structure.html
294-
static constexpr std::array<unsigned char, 8> PNG_SIGNATURE = {
295-
0x89,
296-
0x50,
297-
0x4E,
298-
0x47,
299-
0x0D,
300-
0x0A,
301-
0x1A,
302-
0x0A
303-
};
304-
305-
std::ifstream file(path, std::ios::binary);
306-
if (!file) {
307-
return false;
308-
}
309-
310-
std::array<unsigned char, 8> header;
311-
file.read(reinterpret_cast<char *>(header.data()), 8);
312-
313-
if (file.gcount() != 8) {
314-
return false;
315-
}
316-
317-
return header == PNG_SIGNATURE;
318-
}
319-
320287
/**
321288
* @brief Get the index page.
322289
* @param response The HTTP response object.
@@ -987,8 +954,9 @@ namespace confighttp {
987954
* @api_examples{/api/covers/9999 | GET| null}
988955
*/
989956
void getCover(resp_https_t response, req_https_t request) {
990-
// Skip check_content_type() for this endpoint since the request body is not used.
991-
957+
if (!check_content_type(response, request, "application/json")) {
958+
return;
959+
}
992960
if (!authenticate(response, request)) {
993961
return;
994962
}
@@ -1006,37 +974,22 @@ namespace confighttp {
1006974
auto &apps = file_tree["apps"];
1007975

1008976
auto &app = apps[index];
1009-
if (!app.contains("image-path") || app["image-path"].is_null()) {
1010-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1011-
return;
1012-
}
1013-
1014-
fs::path path = app["image-path"];
1015977

1016-
const fs::path coverdir = platf::appdata().string() + "/covers/";
1017-
if (fs::exists(coverdir / path)) {
1018-
path = coverdir / path;
978+
// Get the image path from the app configuration
979+
std::string app_image_path;
980+
if (app.contains("image-path") && !app["image-path"].is_null()) {
981+
app_image_path = app["image-path"];
1019982
}
1020983

1021-
if (fs::exists(platf::appdata() / path)) {
1022-
path = platf::appdata().string() / path;
1023-
}
1024-
1025-
if (!fs::exists(path) || path.extension() != ".png") {
1026-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1027-
return;
1028-
}
984+
// Use validate_app_image_path to resolve and validate the path
985+
// This handles extension validation, PNG signature validation, and path resolution
986+
std::string validated_path = proc::validate_app_image_path(app_image_path);
1029987

1030-
std::ifstream in(path, std::ios::binary);
988+
// Open and stream the validated file
989+
std::ifstream in(validated_path, std::ios::binary);
1031990
if (!in) {
1032-
BOOST_LOG(debug) << "Unable to read file: " << path;
1033-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1034-
return;
1035-
}
1036-
1037-
if (!check_valid_png(path)) {
1038-
BOOST_LOG(debug) << "User requested corrupted png: " << path;
1039-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
991+
BOOST_LOG(warning) << "Unable to read cover image file: " << validated_path;
992+
bad_request(response, request, "Unable to read cover image file");
1040993
return;
1041994
}
1042995

src/process.cpp

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@
3939
#include <share.h>
4040
#endif
4141

42-
#define DEFAULT_APP_IMAGE_PATH SUNSHINE_ASSETS_DIR "/box.png"
43-
4442
namespace proc {
4543
using namespace std::literals;
4644
namespace pt = boost::property_tree;
@@ -466,6 +464,40 @@ namespace proc {
466464
return ss.str();
467465
}
468466

467+
/**
468+
* @brief Validates a path whether it is a valid PNG.
469+
* @param path The path to the PNG file.
470+
* @return true if the file has a valid PNG signature, false otherwise.
471+
*/
472+
bool check_valid_png(const std::filesystem::path &path) {
473+
// PNG signature as defined in PNG specification
474+
// http://www.libpng.org/pub/png/spec/1.2/PNG-Structure.html
475+
static constexpr std::array<unsigned char, 8> PNG_SIGNATURE = {
476+
0x89,
477+
0x50,
478+
0x4E,
479+
0x47,
480+
0x0D,
481+
0x0A,
482+
0x1A,
483+
0x0A
484+
};
485+
486+
std::ifstream file(path, std::ios::binary);
487+
if (!file) {
488+
return false;
489+
}
490+
491+
std::array<unsigned char, 8> header;
492+
file.read(reinterpret_cast<char *>(header.data()), 8);
493+
494+
if (file.gcount() != 8) {
495+
return false;
496+
}
497+
498+
return header == PNG_SIGNATURE;
499+
}
500+
469501
std::string validate_app_image_path(std::string app_image_path) {
470502
if (app_image_path.empty()) {
471503
return DEFAULT_APP_IMAGE_PATH;
@@ -475,28 +507,39 @@ namespace proc {
475507
auto image_extension = std::filesystem::path(app_image_path).extension().string();
476508
boost::to_lower(image_extension);
477509

478-
// return the default box image if extension is not "png"
510+
// return the default box image if the extension is not "png"
479511
if (image_extension != ".png") {
480512
return DEFAULT_APP_IMAGE_PATH;
481513
}
482514

483515
// check if image is in assets directory
484-
auto full_image_path = std::filesystem::path(SUNSHINE_ASSETS_DIR) / app_image_path;
485-
if (std::filesystem::exists(full_image_path)) {
516+
if (auto full_image_path = std::filesystem::path(SUNSHINE_ASSETS_DIR) / app_image_path; std::filesystem::exists(full_image_path)) {
517+
// Validate PNG signature
518+
if (!check_valid_png(full_image_path)) {
519+
BOOST_LOG(warning) << "Invalid PNG file at path ["sv << full_image_path << ']';
520+
return DEFAULT_APP_IMAGE_PATH;
521+
}
486522
return full_image_path.string();
487-
} else if (app_image_path == "./assets/steam.png") {
523+
}
524+
525+
if (app_image_path == "./assets/steam.png") {
488526
// handle old default steam image definition
489527
return SUNSHINE_ASSETS_DIR "/steam.png";
490528
}
491529

492530
// check if specified image exists
493-
std::error_code code;
494-
if (!std::filesystem::exists(app_image_path, code)) {
531+
if (std::error_code code; !std::filesystem::exists(app_image_path, code)) {
495532
// return default box image if image does not exist
496533
BOOST_LOG(warning) << "Couldn't find app image at path ["sv << app_image_path << ']';
497534
return DEFAULT_APP_IMAGE_PATH;
498535
}
499536

537+
// Validate PNG signature
538+
if (!check_valid_png(app_image_path)) {
539+
BOOST_LOG(warning) << "Invalid PNG file at path ["sv << app_image_path << ']';
540+
return DEFAULT_APP_IMAGE_PATH;
541+
}
542+
500543
// image is a png, and not in assets directory
501544
// return only "content-type" http header compatible image type
502545
return app_image_path;

src/process.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
#include "rtsp.h"
2222
#include "utility.h"
2323

24+
#define DEFAULT_APP_IMAGE_PATH SUNSHINE_ASSETS_DIR "/box.png"
25+
2426
namespace proc {
2527
using file_t = util::safe_ptr_v2<FILE, int, fclose>;
2628

@@ -120,6 +122,7 @@ namespace proc {
120122
*/
121123
std::tuple<std::string, std::string> calculate_app_id(const std::string &app_name, std::string app_image_path, int index);
122124

125+
bool check_valid_png(const std::filesystem::path &path);
123126
std::string validate_app_image_path(std::string app_image_path);
124127
void refresh(const std::string &file_name);
125128
std::optional<proc::proc_t> parse(const std::string &file_name);

0 commit comments

Comments
 (0)