Handle webp_transform input robustly

plugins/webp_transform/ImageTransform.cc

@@ -19,6 +19,7 @@
 #include <sstream>
 #include <iostream>
 #include <string_view>
+#include <utility>
 
 #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<unsigned char>(data[0]) == 0xff && static_cast<unsigned char>(data[1]) == 0xd8 &&
+           static_cast<unsigned char>(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<int>(_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<const char *>(output_blob.data()), output_blob.length()));
     } catch (Magick::Warning &warning) {
       TSError("ImageMagick++ warning: %s", warning.what());
-      produce(std::string_view(reinterpret_cast<const char *>(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<const char *>(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<const char *>(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<const char *>(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");
       }

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}

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')