Fix misuses of set_error() and set_exception() callbacks that could sometimes lead to injection of formatting strings through query parameters

Fixes MapServer/MapServer-private#3

Author: rouault

include/mapcache.h

@@ -148,7 +148,13 @@ struct mapcache_extent_i {
   int maxy;
 };
 
-
+#ifdef __GNUC__
+/** Tag a function to have printf() formatting */
+#define MAPCACHE_PRINT_FUNC_FORMAT(format_idx, arg_idx) \
+    __attribute__((__format__(__printf__, format_idx, arg_idx)))
+#else
+#define MAPCACHE_PRINT_FUNC_FORMAT(format_idx, arg_idx)
+#endif
 
 mapcache_image *mapcache_error_image(mapcache_context *ctx, int width, int height, char *msg);
 
@@ -164,9 +170,9 @@ struct mapcache_context {
    * \param code the error code
    * \param message human readable message of what happened
    */
-  void (*set_error)(mapcache_context *ctx, int code, char *message, ...);
+  void (*set_error)(mapcache_context *ctx, int code, char *message, ...) MAPCACHE_PRINT_FUNC_FORMAT(3, 4);
 
-  void (*set_exception)(mapcache_context *ctx, char *key, char *message, ...);
+  void (*set_exception)(mapcache_context *ctx, char *key, char *message, ...) MAPCACHE_PRINT_FUNC_FORMAT(3, 4);
 
   /**
    * \brief query context to know if an error has occurred

lib/cache_fallback.c

@@ -86,7 +86,7 @@ static int _mapcache_cache_fallback_tile_get(mapcache_context *ctx, mapcache_cac
       }
     }
     /* all backends failed, return primary error message */
-    ctx->set_error(ctx,first_error,first_error_message);
+    ctx->set_error(ctx,first_error,"%s",first_error_message);
     return MAPCACHE_FAILURE;
   } else {
     /* success or notfound */
@@ -113,7 +113,7 @@ static void _mapcache_cache_fallback_tile_set(mapcache_context *ctx, mapcache_ca
     }
   }
   if(first_error) {
-    ctx->set_error(ctx,first_error,first_error_message);
+    ctx->set_error(ctx,first_error,"%s",first_error_message);
   }
 }
 
@@ -136,7 +136,7 @@ static void _mapcache_cache_fallback_tile_multi_set(mapcache_context *ctx, mapca
     }
   }
   if(first_error) {
-    ctx->set_error(ctx,first_error,first_error_message);
+    ctx->set_error(ctx,first_error,"%s",first_error_message);
   }
 }
 

lib/core.c

@@ -151,6 +151,7 @@ void mapcache_prefetch_tiles(mapcache_context *ctx, mapcache_tile **tiles, int n
     if(GC_HAS_ERROR(thread_tiles[i].ctx)) {
       /* transfer error message from child thread to main context */
       ctx->set_error(ctx,thread_tiles[i].ctx->get_error(thread_tiles[i].ctx),
+                     "%s",
                      thread_tiles[i].ctx->get_error_message(thread_tiles[i].ctx));
     }
   }
@@ -597,7 +598,8 @@ mapcache_http_response *mapcache_core_get_featureinfo(mapcache_context *ctx,
     apr_table_set(response->headers,"Content-Type",fi->format);
     return response;
   } else {
-    ctx->set_error(ctx,404, "tileset %s does not support feature info requests");
+    ctx->set_error(ctx,404, "tileset %s does not support feature info requests",
+                   tileset->name);
     return NULL;
   }
 }

lib/service_kml.c

@@ -272,7 +272,7 @@ void _mapcache_service_kml_parse_request(mapcache_context *ctx, mapcache_service
           }
           endptr++;
           if(strcmp(endptr,"kml")) {
-            ctx->set_error(ctx,404, "received kml request with invalid extension %s", pathinfo, endptr);
+            ctx->set_error(ctx,404, "received kml request %s with invalid extension %s", pathinfo, endptr);
             return;
           }
           break;

lib/service_mapguide.c

@@ -217,7 +217,7 @@ void _mapcache_service_mg_parse_request(mapcache_context *ctx, mapcache_service
     *request = (mapcache_request*)req;
     return;
   } else {
-    ctx->set_error(ctx,404, "received request with wrong number of arguments", pathinfo);
+    ctx->set_error(ctx,404, "received request %s with wrong number of arguments", pathinfo);
     return;
   }
 }

lib/service_tms.c

@@ -461,7 +461,7 @@ void _mapcache_service_tms_parse_request(mapcache_context *ctx, mapcache_service
     *request = (mapcache_request*)req;
     return;
   } else {
-    ctx->set_error(ctx,404, "received request with wrong number of arguments", pathinfo);
+    ctx->set_error(ctx,404, "received request %s with wrong number of arguments", pathinfo);
     return;
   }
 }

lib/service_wms.c

@@ -958,7 +958,7 @@ void _mapcache_service_wms_parse_request(mapcache_context *ctx, mapcache_service
    * or there was an error parsing the request and no rules to proxy it elsewhere
    */
   if(errcode != 200) {
-    ctx->set_error(ctx,errcode,errmsg);
+    ctx->set_error(ctx,errcode,"%s",errmsg);
     return;
   }
 #ifdef DEBUG

lib/service_wmts.c

@@ -905,7 +905,7 @@ void _mapcache_service_wmts_parse_request(mapcache_context *ctx, mapcache_servic
     req->tiles[0] = mapcache_tileset_tile_create(ctx->pool, tileset, grid_link);
     if(!req->tiles[0]) {
       ctx->set_error(ctx, 500, "failed to allocate tile");
-      if(kvp) ctx->set_exception(ctx,"NoApplicableCode","");
+      if(kvp) ctx->set_exception(ctx,"NoApplicableCode","%s","");
       return;
     }
 
@@ -953,7 +953,7 @@ void _mapcache_service_wmts_parse_request(mapcache_context *ctx, mapcache_servic
 
     if(!tileset->source || !tileset->source->info_formats) {
       ctx->set_error(ctx,400,"tileset %s does not support featureinfo requests", tileset->name);
-      if(kvp) ctx->set_exception(ctx,"OperationNotSupported","");
+      if(kvp) ctx->set_exception(ctx,"OperationNotSupported","%s","");
       return;
     }
     req_fi = (mapcache_request_get_feature_info*)apr_pcalloc(

lib/source_fallback.c

@@ -72,7 +72,7 @@ void _mapcache_source_fallback_render_map(mapcache_context *ctx, mapcache_source
       }
     }
     /* all backends failed, return primary error message */
-    ctx->set_error(ctx,first_error,first_error_message);
+    ctx->set_error(ctx,first_error,"%s",first_error_message);
     return;
   }
 }
@@ -106,7 +106,7 @@ void _mapcache_source_fallback_query(mapcache_context *ctx, mapcache_source *pso
       }
     }
     /* all backends failed, return primary error message */
-    ctx->set_error(ctx,first_error,first_error_message);
+    ctx->set_error(ctx,first_error,"%s",first_error_message);
     return;
   }
   ctx->set_error(ctx,500,"fallback source does not support queries");

lib/tileset.c

@@ -964,7 +964,7 @@ void mapcache_tileset_tile_set_get_with_subdimensions(mapcache_context *ctx, map
       if(GC_HAS_ERROR(thread_subtiles[i].ctx)) {                                           
         /* transfer error message from child thread to main context */                     
         ctx->set_error(ctx,thread_subtiles[i].ctx->get_error(thread_subtiles[i].ctx),      
-            thread_subtiles[i].ctx->get_error_message(thread_subtiles[i].ctx));            
+            "%s",thread_subtiles[i].ctx->get_error_message(thread_subtiles[i].ctx));            
       }                                                                                    
     }                                                                                      
   }