Skip to content

Commit d3de613

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 0e8dfad commit d3de613

4 files changed

Lines changed: 341 additions & 69 deletions

File tree

src/confighttp.cpp

Lines changed: 14 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -277,39 +277,6 @@ namespace confighttp {
277277
return true;
278278
}
279279

280-
/**
281-
* @brief Validates a path whether it is a valid png.
282-
* @param path The path to the png file.
283-
*/
284-
bool check_valid_png(fs::path path) {
285-
// PNG signature as defined in PNG specification
286-
// http://www.libpng.org/pub/png/spec/1.2/PNG-Structure.html
287-
static constexpr std::array<unsigned char, 8> PNG_SIGNATURE = {
288-
0x89,
289-
0x50,
290-
0x4E,
291-
0x47,
292-
0x0D,
293-
0x0A,
294-
0x1A,
295-
0x0A
296-
};
297-
298-
std::ifstream file(path, std::ios::binary);
299-
if (!file) {
300-
return false;
301-
}
302-
303-
std::array<unsigned char, 8> header;
304-
file.read(reinterpret_cast<char *>(header.data()), 8);
305-
306-
if (file.gcount() != 8) {
307-
return false;
308-
}
309-
310-
return header == PNG_SIGNATURE;
311-
}
312-
313280
/**
314281
* @brief Get the index page.
315282
* @param response The HTTP response object.
@@ -980,8 +947,9 @@ namespace confighttp {
980947
* @api_examples{/api/covers/9999 | GET| null}
981948
*/
982949
void getCover(resp_https_t response, req_https_t request) {
983-
// Skip check_content_type() for this endpoint since the request body is not used.
984-
950+
if (!check_content_type(response, request, "application/json")) {
951+
return;
952+
}
985953
if (!authenticate(response, request)) {
986954
return;
987955
}
@@ -999,37 +967,22 @@ namespace confighttp {
999967
auto &apps = file_tree["apps"];
1000968

1001969
auto &app = apps[index];
1002-
if (!app.contains("image-path") || app["image-path"].is_null()) {
1003-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1004-
return;
1005-
}
1006-
1007-
fs::path path = app["image-path"];
1008970

1009-
const fs::path coverdir = platf::appdata().string() + "/covers/";
1010-
if (fs::exists(coverdir / path)) {
1011-
path = coverdir / path;
971+
// Get the image path from the app configuration
972+
std::string app_image_path;
973+
if (app.contains("image-path") && !app["image-path"].is_null()) {
974+
app_image_path = app["image-path"];
1012975
}
1013976

1014-
if (fs::exists(platf::appdata() / path)) {
1015-
path = platf::appdata().string() / path;
1016-
}
1017-
1018-
if (!fs::exists(path) || path.extension() != ".png") {
1019-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1020-
return;
1021-
}
977+
// Use validate_app_image_path to resolve and validate the path
978+
// This handles extension validation, PNG signature validation, and path resolution
979+
std::string validated_path = proc::validate_app_image_path(app_image_path);
1022980

1023-
std::ifstream in(path, std::ios::binary);
981+
// Open and stream the validated file
982+
std::ifstream in(validated_path, std::ios::binary);
1024983
if (!in) {
1025-
BOOST_LOG(debug) << "Unable to read file: " << path;
1026-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
1027-
return;
1028-
}
1029-
1030-
if (!check_valid_png(path)) {
1031-
BOOST_LOG(debug) << "User requested corrupted png: " << path;
1032-
not_found(response, request, "'image-path' not set or does not have a 'png' file extension");
984+
BOOST_LOG(warning) << "Unable to read cover image file: " << validated_path;
985+
bad_request(response, request, "Unable to read cover image file");
1033986
return;
1034987
}
1035988

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)