diff --git a/plugins/webp_transform/ImageTransform.cc b/plugins/webp_transform/ImageTransform.cc index 215567183c6..809a5e5e989 100644 --- a/plugins/webp_transform/ImageTransform.cc +++ b/plugins/webp_transform/ImageTransform.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include "ts/ts.h" @@ -58,40 +59,75 @@ bool config_convert_to_jpeg = false; Stat stat_convert_to_webp; Stat stat_convert_to_jpeg; + +bool +has_signature_for(std::string_view data, ImageEncoding encoding) +{ + constexpr std::string_view png_signature{"\x89PNG\r\n\x1a\n", 8}; + + switch (encoding) { + case ImageEncoding::webp: + return data.size() >= 12 && data.substr(0, 4) == "RIFF" && data.substr(8, 4) == "WEBP"; + case ImageEncoding::jpeg: + return data.size() >= 3 && static_cast(data[0]) == 0xff && static_cast(data[1]) == 0xd8 && + static_cast(data[2]) == 0xff; + case ImageEncoding::png: + return data.starts_with(png_signature); + case ImageEncoding::unknown: + return false; + } + + return false; +} } // namespace class ImageTransform : public TransformationPlugin { public: - ImageTransform(Transaction &transaction, ImageEncoding input_image_type, ImageEncoding transform_image_type) + ImageTransform(Transaction &transaction, std::string input_content_type, ImageEncoding input_image_type, + ImageEncoding transform_image_type) : TransformationPlugin(transaction, TransformationPlugin::RESPONSE_TRANSFORMATION), + _input_content_type(std::move(input_content_type)), _input_image_type(input_image_type), _transform_image_type(transform_image_type) { TransformationPlugin::registerHook(HOOK_READ_RESPONSE_HEADERS); + TransformationPlugin::registerHook(HOOK_SEND_RESPONSE_HEADERS); } void handleReadResponseHeaders(Transaction &transaction) override { + transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // to have a separate cache entry + + Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); + transaction.resume(); + } + + void + handleSendResponseHeaders(Transaction &transaction) override + { + if (_transform_image_type == _input_image_type) { + transaction.getClientResponse().getHeaders()["Content-Type"] = _input_content_type; + transaction.resume(); + return; + } + switch (_transform_image_type) { case ImageEncoding::webp: - transaction.getServerResponse().getHeaders()["Content-Type"] = "image/webp"; + transaction.getClientResponse().getHeaders()["Content-Type"] = "image/webp"; break; case ImageEncoding::jpeg: - transaction.getServerResponse().getHeaders()["Content-Type"] = "image/jpeg"; + transaction.getClientResponse().getHeaders()["Content-Type"] = "image/jpeg"; break; case ImageEncoding::png: - transaction.getServerResponse().getHeaders()["Content-Type"] = "image/png"; + transaction.getClientResponse().getHeaders()["Content-Type"] = "image/png"; break; case ImageEncoding::unknown: // do nothing break; } - transaction.getServerResponse().getHeaders()["Vary"] = "Accept"; // to have a separate cache entry - - Dbg(webp_dbg_ctl, "url %s", transaction.getServerRequest().getUrl().getUrlString().c_str()); transaction.resume(); } @@ -105,8 +141,17 @@ class ImageTransform : public TransformationPlugin handleInputComplete() override { std::string input_data = _img.str(); - Blob input_blob(input_data.data(), input_data.length()); - Image image; + + if (!has_signature_for(input_data, _input_image_type)) { + TSError("[webp_transform] input body does not match its declared image encoding: %d, length: %zu", + static_cast(_input_image_type), input_data.length()); + pass_through(input_data); + setOutputComplete(); + return; + } + + Blob input_blob(input_data.data(), input_data.length()); + Image image; try { image.read(input_blob); @@ -125,13 +170,11 @@ class ImageTransform : public TransformationPlugin produce(std::string_view(reinterpret_cast(output_blob.data()), output_blob.length())); } catch (Magick::Warning &warning) { TSError("ImageMagick++ warning: %s", warning.what()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } catch (Magick::Error &error) { - TSError("ImageMagick++ error: %s _image_type: %d input_data.length(): %zd", error.what(), (int)_transform_image_type, + TSError("ImageMagick++ error: %s _image_type: %d input_data.length(): %zu", error.what(), (int)_transform_image_type, input_data.length()); - produce(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); - _transform_image_type = _input_image_type; // Revert to original encoding on error + pass_through(std::string_view(reinterpret_cast(input_blob.data()), input_blob.length())); } setOutputComplete(); @@ -140,7 +183,17 @@ class ImageTransform : public TransformationPlugin ~ImageTransform() override = default; private: + void + pass_through(std::string_view data) + { + if (!data.empty()) { + produce(data); + } + _transform_image_type = _input_image_type; + } + std::stringstream _img; + std::string _input_content_type; ImageEncoding _input_image_type; ImageEncoding _transform_image_type; }; @@ -192,10 +245,10 @@ class GlobalHookPlugin : public GlobalPlugin if (webp_supported == true && transaction_convert_to_webp == true) { Dbg(webp_dbg_ctl, "Content type is either jpeg or png. Converting to webp"); - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::webp)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::webp)); } else if (webp_supported == false && transaction_convert_to_jpeg == true) { Dbg(webp_dbg_ctl, "Content type is webp. Converting to jpeg"); - transaction.addPlugin(new ImageTransform(transaction, input_image_type, ImageEncoding::jpeg)); + transaction.addPlugin(new ImageTransform(transaction, ctype, input_image_type, ImageEncoding::jpeg)); } else { Dbg(webp_dbg_ctl, "Nothing to convert"); } diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml new file mode 100644 index 00000000000..ccc8cc5de24 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.replay.yaml @@ -0,0 +1,196 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +meta: + version: "1.0" + +autest: + description: 'Verify webp_transform safely passes through invalid image bodies' + + server: + name: 'webp-transform-server' + + client: + name: 'webp-transform-client' + + ats: + name: 'webp-transform-ts' + process_config: + enable_cache: false + disable_log_checks: true + + plugin_config: + - 'webp_transform.so convert_to_jpeg,convert_to_webp' + + records_config: + proxy.config.diags.debug.enabled: 1 + proxy.config.diags.debug.tags: 'webp_transform' + + remap_config: + - from: 'http://www.example.com/' + to: 'http://127.0.0.1:{SERVER_HTTP_PORT}/' + + log_validation: + diags_log: + contains: + - expression: 'input body does not match its declared image encoding' + description: 'Verify invalid input is rejected before conversion' + excludes: + - expression: 'zero-length blob not permitted|no decode delegate for this image format' + description: 'Verify ImageMagick never receives the invalid input' + +sessions: + - transactions: + - client-request: + method: GET + version: '1.1' + url: /empty.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, empty-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 0] + content: + size: 0 + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + size: 0 + + - client-request: + method: GET + version: '1.1' + url: /invalid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, invalid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 1] + content: + data: x + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/webp, as: equal}] + content: + verify: {value: x, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /invalid.jpg + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/webp'] + - [uuid, invalid-jpeg] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/jpeg] + - [Content-Length, 8] + content: + data: not-jpeg + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + content: + verify: {value: not-jpeg, as: equal} + + - client-request: + method: GET + version: '1.1' + url: /valid.webp + headers: + fields: + - [Host, www.example.com] + - [Accept, 'image/jpeg'] + - [uuid, valid-webp] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, image/webp] + - [Content-Length, 68] + content: + encoding: uri + data: "%52%49%46%46%3c%00%00%00%57%45%42%50%56%50%38%20%30%00%00%00%d0%01%00%9d\ + %01%2a%01%00%01%00%02%00%34%25%a0%02%74%ba%01%f8%00%03%b0%00%fe%f0%c4%0b\ + %ff%20%b9%61%75%c8%d7%ff%20%3f%e4%07%fc%80%ff%f8%f2%00%00%00" + + proxy-response: + status: 200 + headers: + fields: + - [Content-Type, {value: image/jpeg, as: equal}] + - [Vary, {value: Accept, as: equal}] + content: + verify: {value: JFIF, as: contains} + + - client-request: + method: GET + version: '1.1' + url: /health + headers: + fields: + - [Host, www.example.com] + - [uuid, health] + + server-response: + status: 200 + reason: OK + headers: + fields: + - [Content-Type, text/plain] + - [Content-Length, 2] + content: + data: OK + + proxy-response: + status: 200 + content: + verify: {value: OK, as: equal} diff --git a/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py new file mode 100644 index 00000000000..e5c4961b909 --- /dev/null +++ b/tests/gold_tests/pluginTest/webp_transform/webp_transform_invalid_input.test.py @@ -0,0 +1,25 @@ +''' +Verify webp_transform rejects invalid image input before invoking ImageMagick. +''' +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +Test.Summary = 'Verify webp_transform safely passes through invalid image bodies' + +Test.SkipUnless(Condition.PluginExists('webp_transform.so')) + +Test.ATSReplayTest(replay_file='webp_transform_invalid_input.replay.yaml')