You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-commits@axis.apache.org by ro...@apache.org on 2026/08/25 13:53:01 UTC
(axis-axis2-c-core) 07/08: Fix XPath expression ownership and token buffer bounds
This is an automated email from the ASF dual-hosted git repository.
robertlazarski pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/axis-axis2-c-core.git
commit 5121c91577de72b2efbdf237a8908608b0e8d3ca
Author: Robert Lazarski <ro...@gmail.com>
AuthorDate: Tue Aug 25 03:23:51 2026 -1000
Fix XPath expression ownership and token buffer bounds
The context freed the caller's compiled expression on attach and again
on free, so re-evaluating one used it after free; it now borrows.
Literal and number operators aliased the expression's parameters into
result nodes that evaluation frees. NCName and literal copies had no
bound on their 255-byte frame buffers.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
axiom/src/soap/soap_builder.c | 18 +++++++++++++---
axiom/src/xpath/xpath.c | 13 ++++++-----
axiom/src/xpath/xpath_internals.c | 17 ++++++++++++---
axiom/src/xpath/xpath_internals_engine.c | 37 +++++++++++++++++++++++++++-----
axiom/src/xpath/xpath_internals_parser.c | 16 ++++++++++++++
5 files changed, 83 insertions(+), 18 deletions(-)
diff --git a/axiom/src/soap/soap_builder.c b/axiom/src/soap/soap_builder.c
index 7bba3b88f..640b8a06f 100644
--- a/axiom/src/soap/soap_builder.c
+++ b/axiom/src/soap/soap_builder.c
@@ -1042,6 +1042,12 @@ axiom_soap_builder_set_mime_body_parts(
const axutil_env_t * env,
axutil_hash_t * map)
{
+ /* Callers reach here after a multipart body has been parsed, which is
+ * decided from the Content-Type independently of whether that same header
+ * selected a builder. The two can disagree, so a NULL builder is a state
+ * the wire can produce, not a programming error. */
+ AXIS2_PARAM_CHECK(env->error, soap_builder, AXIS2_FAILURE);
+
soap_builder->mime_body_parts = map;
return AXIS2_SUCCESS;
}
@@ -1193,9 +1199,15 @@ axiom_soap_builder_replace_xop(
data_handler, &data_om_node);
axiom_text_set_content_id(data_text, env, id_decoded);
- /*axiom_stax_builder_set_lastnode
- (soap_builder->om_builder, env,
- parent);*/
+ /* The free_tree above can have taken the
+ * builder's lastnode with it -- it points
+ * into the subtree that just went away.
+ * Repoint it at the surviving parent, which
+ * is what the streaming path does after the
+ * identical free. This call was commented
+ * out, leaving the cursor dangling. */
+ axiom_stax_builder_set_lastnode(builder->om_builder, env,
+ parent);
is_replaced = AXIS2_TRUE;
}
if(id_decoded)
diff --git a/axiom/src/xpath/xpath.c b/axiom/src/xpath/xpath.c
index ce919bace..58003e0bf 100755
--- a/axiom/src/xpath/xpath.c
+++ b/axiom/src/xpath/xpath.c
@@ -372,13 +372,12 @@ axiom_xpath_free_context(
{
if (context)
{
- /* Free the expression if not freed */
- if (context->expr)
- {
- axiom_xpath_free_expression(env, context->expr);
-
- context->expr = NULL;
- }
+ /* The expression is borrowed, not owned -- see the note in
+ * axiom_xpath_expression_copy. Freeing it here made the documented
+ * sequence compile -> evaluate -> free_expression -> free_context a
+ * double free, since the caller frees what it compiled. Drop the
+ * reference and leave the object to its owner. */
+ context->expr = NULL;
if (context->root_node)
{
diff --git a/axiom/src/xpath/xpath_internals.c b/axiom/src/xpath/xpath_internals.c
index f71d32abd..2d65ea2c7 100755
--- a/axiom/src/xpath/xpath_internals.c
+++ b/axiom/src/xpath/xpath_internals.c
@@ -18,7 +18,18 @@
#include <axiom_xpath.h>
#include "xpath_internals.h"
-/* Make a copy of the xpath expression */
+/* Attach the expression to the context.
+ *
+ * The name says copy; nothing is copied, and nothing should be freed here
+ * either. The context borrows the expression -- the caller compiled it, the
+ * caller owns it, and axiom_xpath_free_expression is public API for exactly
+ * that. Freeing the previous expression at this point had two ways to go
+ * wrong: evaluating the same expression twice on one context freed it and then
+ * walked the operations list below, and evaluating a second expression freed
+ * the first one out from under a caller still holding it. The matching half of
+ * this is axiom_xpath_free_context, which no longer frees the expression
+ * either.
+ */
void
axiom_xpath_expression_copy(
axiom_xpath_context_t *context,
@@ -27,8 +38,8 @@ axiom_xpath_expression_copy(
int i;
axiom_xpath_operation_t *op;
- if (context->expr)
- axiom_xpath_free_expression(context->env, context->expr);
+ if (!expr)
+ return;
context->expr = expr;
diff --git a/axiom/src/xpath/xpath_internals_engine.c b/axiom/src/xpath/xpath_internals_engine.c
index a6555b323..af7c3d74a 100644
--- a/axiom/src/xpath/xpath_internals_engine.c
+++ b/axiom/src/xpath/xpath_internals_engine.c
@@ -185,7 +185,13 @@ axiom_xpath_literal_operator(
/* This is not required; it gives some problems */
/* context->node = NULL;*/
- node->value = op->par1;
+ /* Copy, do not alias. op->par1 belongs to the compiled expression and is
+ * freed by axiom_xpath_free_expression. Result nodes of type TEXT are
+ * treated as owning their value -- AXIOM_XPATH_CAST_SET_VALUE and the
+ * predicate evaluator both free node->value for non-node types -- so
+ * pushing the expression's own pointer here had evaluation free it, and
+ * then the expression free it again. Every result node owns its value. */
+ node->value = axutil_strdup(context->env, (axis2_char_t *)op->par1);
node->type = AXIOM_XPATH_TYPE_TEXT;
axutil_stack_push(context->stack, context->env, node);
@@ -207,7 +213,13 @@ axiom_xpath_number_operator(
/* This is not required; it gives some problems */
/* context->node = NULL;*/
- node->value = op->par1;
+ /* Same ownership rule as the literal operator above: par1 here is the
+ * compiled expression's double, so hand the stack a copy of it. */
+ node->value = AXIS2_MALLOC(context->env->allocator, sizeof(double));
+ if(node->value)
+ {
+ *((double *)node->value) = *((double *)op->par1);
+ }
node->type = AXIOM_XPATH_TYPE_NUMBER;
axutil_stack_push(context->stack, context->env, node);
@@ -284,7 +296,12 @@ axiom_xpath_orexpr_operator(
{
axiom_xpath_result_node_t *node;
axiom_xpath_result_node_t *res_node;
- int n_nodes[2];
+ /* Both entries must be defined before the pop loops below use them as
+ * bounds. An operand that the parser ended early is skipped by the
+ * evaluation loop, which then leaves its count unwritten -- a truncated
+ * expression such as a trailing '=' produces exactly that shape, and the
+ * pop loop ran against whatever was on the stack. */
+ int n_nodes[2] = { 0, 0 };
int i, j;
int op12[2];
axutil_array_list_t *arr[2];
@@ -358,7 +375,12 @@ axiom_xpath_andexpr_operator(
{
axiom_xpath_result_node_t *node;
axiom_xpath_result_node_t *res_node;
- int n_nodes[2];
+ /* Both entries must be defined before the pop loops below use them as
+ * bounds. An operand that the parser ended early is skipped by the
+ * evaluation loop, which then leaves its count unwritten -- a truncated
+ * expression such as a trailing '=' produces exactly that shape, and the
+ * pop loop ran against whatever was on the stack. */
+ int n_nodes[2] = { 0, 0 };
int i, j;
int op12[2];
axutil_array_list_t *arr[2];
@@ -432,7 +454,12 @@ axiom_xpath_equalexpr_operator(
{
axiom_xpath_result_node_t *node;
axiom_xpath_result_node_t *res_node;
- int n_nodes[2];
+ /* Both entries must be defined before the pop loops below use them as
+ * bounds. An operand that the parser ended early is skipped by the
+ * evaluation loop, which then leaves its count unwritten -- a truncated
+ * expression such as a trailing '=' produces exactly that shape, and the
+ * pop loop ran against whatever was on the stack. */
+ int n_nodes[2] = { 0, 0 };
int i, j;
int op12[2];
axutil_array_list_t *arr[2];
diff --git a/axiom/src/xpath/xpath_internals_parser.c b/axiom/src/xpath/xpath_internals_parser.c
index c3f58c71d..92822f444 100755
--- a/axiom/src/xpath/xpath_internals_parser.c
+++ b/axiom/src/xpath/xpath_internals_parser.c
@@ -968,6 +968,13 @@ axiom_xpath_compile_literal(
while(AXIOM_XPATH_HAS_MORE && AXIOM_XPATH_CURRENT != del)
{
+ /* Same fixed frame buffer, same unbounded source: everything up to the
+ * closing quote was copied in regardless of how much of it there was.
+ * Refuse a literal that does not fit rather than write past lit. */
+ if(i >= (int)sizeof(lit) - 1)
+ {
+ return NULL;
+ }
lit[i] = AXIOM_XPATH_CURRENT;
AXIOM_XPATH_READ(1);
++i;
@@ -1133,6 +1140,15 @@ axiom_xpath_compile_ncname(
while(AXIOM_XPATH_HAS_MORE && (isalnum(AXIOM_XPATH_CURRENT) || AXIOM_XPATH_CURRENT == '_'
|| AXIOM_XPATH_CURRENT == '.' || AXIOM_XPATH_CURRENT == '-'))
{
+ /* name is a fixed frame buffer and the token length comes from the
+ * expression text, so the copy has to stop at the buffer rather than
+ * at the end of the name. A name that does not fit is a name this
+ * parser cannot represent -- refuse it instead of truncating to
+ * something that would silently match a different node. */
+ if(i >= (int)sizeof(name) - 1)
+ {
+ return NULL;
+ }
name[i] = AXIOM_XPATH_CURRENT;
AXIOM_XPATH_READ(1);
++i;