Skip to content

Commit 3a73f9c

Browse files
committed
More hardening of XML output against invalid entities
This replaces the add node function with the one intended for text nodes to prevent that invalid entities are not encoded and therefore could break the XML output. (See d739c65 and #2255)
1 parent 4c57532 commit 3a73f9c

File tree

3 files changed

+33
-33
lines changed

3 files changed

+33
-33
lines changed

src/admin.c

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ xmlDocPtr admin_build_sourcelist(const char *mount)
229229
xmlDocSetRootElement(doc, xmlnode);
230230

231231
if (mount) {
232-
xmlNewChild (xmlnode, NULL, XMLSTR("current_source"), XMLSTR(mount));
232+
xmlNewTextChild (xmlnode, NULL, XMLSTR("current_source"), XMLSTR(mount));
233233
}
234234

235235
node = avl_get_first(global.source_tree);
@@ -250,11 +250,11 @@ xmlDocPtr admin_build_sourcelist(const char *mount)
250250
srcnode = xmlNewChild(xmlnode, NULL, XMLSTR("source"), NULL);
251251
xmlSetProp(srcnode, XMLSTR("mount"), XMLSTR(source->mount));
252252

253-
xmlNewChild(srcnode, NULL, XMLSTR("fallback"),
253+
xmlNewTextChild(srcnode, NULL, XMLSTR("fallback"),
254254
(source->fallback_mount != NULL)?
255255
XMLSTR(source->fallback_mount):XMLSTR(""));
256256
snprintf(buf, sizeof(buf), "%lu", source->listeners);
257-
xmlNewChild(srcnode, NULL, XMLSTR("listeners"), XMLSTR(buf));
257+
xmlNewTextChild(srcnode, NULL, XMLSTR("listeners"), XMLSTR(buf));
258258

259259
config = config_get_config();
260260
mountinfo = config_find_mount(config, source->mount, MOUNT_TYPE_NORMAL);
@@ -263,7 +263,7 @@ xmlDocPtr admin_build_sourcelist(const char *mount)
263263
if (!acl)
264264
acl = auth_stack_get_anonymous_acl(config->authstack, httpp_req_get);
265265
if (acl && acl_test_web(acl) == ACL_POLICY_DENY) {
266-
xmlNewChild(srcnode, NULL, XMLSTR("authenticator"), XMLSTR("(dummy)"));
266+
xmlNewTextChild(srcnode, NULL, XMLSTR("authenticator"), XMLSTR("(dummy)"));
267267
}
268268
acl_release(acl);
269269
config_release_config();
@@ -272,9 +272,9 @@ xmlDocPtr admin_build_sourcelist(const char *mount)
272272
if (source->client) {
273273
snprintf(buf, sizeof(buf), "%lu",
274274
(unsigned long)(now - source->con->con_time));
275-
xmlNewChild(srcnode, NULL, XMLSTR("Connected"), XMLSTR(buf));
275+
xmlNewTextChild(srcnode, NULL, XMLSTR("Connected"), XMLSTR(buf));
276276
}
277-
xmlNewChild(srcnode, NULL, XMLSTR("content-type"),
277+
xmlNewTextChild(srcnode, NULL, XMLSTR("content-type"),
278278
XMLSTR(source->format->contenttype));
279279
}
280280
}
@@ -646,8 +646,8 @@ static void command_move_clients(client_t *client,
646646

647647
snprintf(buf, sizeof(buf), "Clients moved from %s to %s",
648648
source->mount, dest_source);
649-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
650-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
649+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
650+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
651651

652652
admin_send_response(doc, client, response, ADMIN_XSL_RESPONSE);
653653
xmlFreeDoc(doc);
@@ -674,9 +674,9 @@ static inline xmlNodePtr __add_listener(client_t *client,
674674
memset(buf, '\000', sizeof(buf));
675675
snprintf(buf, sizeof(buf)-1, "%lu", client->con->id);
676676
xmlSetProp(node, XMLSTR("id"), XMLSTR(buf));
677-
xmlNewChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "ID" : "id"), XMLSTR(buf));
677+
xmlNewTextChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "ID" : "id"), XMLSTR(buf));
678678

679-
xmlNewChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "IP" : "ip"), XMLSTR(client->con->ip));
679+
xmlNewTextChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "IP" : "ip"), XMLSTR(client->con->ip));
680680

681681
tmp = httpp_getvar(client->parser, "user-agent");
682682
if (tmp)
@@ -687,7 +687,7 @@ static inline xmlNodePtr __add_listener(client_t *client,
687687
xmlNewTextChild(node, NULL, XMLSTR("referer"), XMLSTR(tmp));
688688

689689
snprintf(buf, sizeof(buf), "%lu", (unsigned long)(now - client->con->con_time));
690-
xmlNewChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "Connected" : "connected"), XMLSTR(buf));
690+
xmlNewTextChild(node, NULL, XMLSTR(mode == OMODE_LEGACY ? "Connected" : "connected"), XMLSTR(buf));
691691

692692
if (client->username)
693693
xmlNewTextChild(node, NULL, XMLSTR("username"), XMLSTR(client->username));
@@ -696,9 +696,9 @@ static inline xmlNodePtr __add_listener(client_t *client,
696696
xmlNewTextChild(node, NULL, XMLSTR("role"), XMLSTR(client->role));
697697

698698
#ifdef HAVE_OPENSSL
699-
xmlNewChild(node, NULL, XMLSTR("tls"), XMLSTR(client->con->ssl ? "true" : "false"));
699+
xmlNewTextChild(node, NULL, XMLSTR("tls"), XMLSTR(client->con->ssl ? "true" : "false"));
700700
#else
701-
xmlNewChild(node, NULL, XMLSTR("tls"), XMLSTR("false"));
701+
xmlNewTextChild(node, NULL, XMLSTR("tls"), XMLSTR("false"));
702702
#endif
703703

704704
return node;
@@ -737,7 +737,7 @@ static void command_show_listeners(client_t *client,
737737
memset(buf, '\000', sizeof(buf));
738738
snprintf (buf, sizeof(buf), "%lu", source->listeners);
739739
/* BEFORE RELEASE NEXT DOCUMENT #2097: Changed "Listeners" to lower case. */
740-
xmlNewChild(srcnode, NULL, XMLSTR(client->mode == OMODE_LEGACY ? "Listeners" : "listeners"), XMLSTR(buf));
740+
xmlNewTextChild(srcnode, NULL, XMLSTR(client->mode == OMODE_LEGACY ? "Listeners" : "listeners"), XMLSTR(buf));
741741

742742
admin_add_listeners_to_mount(source, srcnode, client->mode);
743743

@@ -905,7 +905,7 @@ static void command_manageauth(client_t *client, int response)
905905

906906
if (message) {
907907
msgnode = xmlNewChild(node, NULL, XMLSTR("iceresponse"), NULL);
908-
xmlNewChild(msgnode, NULL, XMLSTR("message"), XMLSTR(message));
908+
xmlNewTextChild(msgnode, NULL, XMLSTR("message"), XMLSTR(message));
909909
}
910910

911911
xmlDocSetRootElement(doc, node);
@@ -939,8 +939,8 @@ static void command_kill_source(client_t *client,
939939

940940
doc = xmlNewDoc (XMLSTR("1.0"));
941941
node = xmlNewDocNode(doc, NULL, XMLSTR("iceresponse"), NULL);
942-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR("Source Removed"));
943-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
942+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR("Source Removed"));
943+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
944944
xmlDocSetRootElement(doc, node);
945945

946946
source->running = 0;
@@ -981,14 +981,14 @@ static void command_kill_client(client_t *client,
981981
listener->con->error = 1;
982982
memset(buf, '\000', sizeof(buf));
983983
snprintf(buf, sizeof(buf)-1, "Client %d removed", id);
984-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
985-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
984+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
985+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
986986
}
987987
else {
988988
memset(buf, '\000', sizeof(buf));
989989
snprintf(buf, sizeof(buf)-1, "Client %d not found", id);
990-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
991-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("0"));
990+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR(buf));
991+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("0"));
992992
}
993993
admin_send_response(doc, client, response,
994994
ADMIN_XSL_RESPONSE);
@@ -1042,8 +1042,8 @@ static void command_metadata(client_t *client,
10421042
COMMAND_OPTIONAL(client, "charset", charset);
10431043

10441044
if (strcmp (action, "updinfo") != 0) {
1045-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR("No such action"));
1046-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("0"));
1045+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR("No such action"));
1046+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("0"));
10471047
admin_send_response(doc, client, response, ADMIN_XSL_RESPONSE);
10481048
xmlFreeDoc(doc);
10491049
return;
@@ -1069,17 +1069,17 @@ static void command_metadata(client_t *client,
10691069
/* updates are now done, let them be pushed into the stream */
10701070
plugin->set_tag (plugin, NULL, NULL, NULL);
10711071
} else {
1072-
xmlNewChild(node, NULL, XMLSTR("message"),
1072+
xmlNewTextChild(node, NULL, XMLSTR("message"),
10731073
XMLSTR("Mountpoint will not accept URL updates"));
1074-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
1074+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
10751075
admin_send_response(doc, client, response,
10761076
ADMIN_XSL_RESPONSE);
10771077
xmlFreeDoc(doc);
10781078
return;
10791079
}
10801080

1081-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR("Metadata update successful"));
1082-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
1081+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR("Metadata update successful"));
1082+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
10831083
admin_send_response(doc, client, response, ADMIN_XSL_RESPONSE);
10841084
xmlFreeDoc(doc);
10851085
}
@@ -1143,8 +1143,8 @@ static void command_queue_reload(client_t *client, int response)
11431143

11441144
doc = xmlNewDoc (XMLSTR("1.0"));
11451145
node = xmlNewDocNode(doc, NULL, XMLSTR("iceresponse"), NULL);
1146-
xmlNewChild(node, NULL, XMLSTR("message"), XMLSTR("Config reload queued"));
1147-
xmlNewChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
1146+
xmlNewTextChild(node, NULL, XMLSTR("message"), XMLSTR("Config reload queued"));
1147+
xmlNewTextChild(node, NULL, XMLSTR("return"), XMLSTR("1"));
11481148
xmlDocSetRootElement(doc, node);
11491149

11501150
admin_send_response(doc, client, response, ADMIN_XSL_RESPONSE);

src/auth_htpasswd.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,8 +390,8 @@ static auth_result htpasswd_userlist(auth_t *auth, xmlNodePtr srcnode)
390390
while (node) {
391391
htpasswd_user *user = (htpasswd_user *)node->key;
392392
newnode = xmlNewChild(srcnode, NULL, XMLSTR("user"), NULL);
393-
xmlNewChild(newnode, NULL, XMLSTR("username"), XMLSTR(user->name));
394-
xmlNewChild(newnode, NULL, XMLSTR("password"), XMLSTR(user->pass));
393+
xmlNewTextChild(newnode, NULL, XMLSTR("username"), XMLSTR(user->name));
394+
xmlNewTextChild(newnode, NULL, XMLSTR("password"), XMLSTR(user->pass));
395395
node = avl_get_next(node);
396396
}
397397
thread_rwlock_unlock(&state->file_rwlock);

src/auth_static.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ static auth_result static_userlist(auth_t *auth, xmlNodePtr srcnode)
6767
xmlNodePtr newnode;
6868

6969
newnode = xmlNewChild(srcnode, NULL, XMLSTR("user"), NULL);
70-
xmlNewChild(newnode, NULL, XMLSTR("username"), XMLSTR(auth_info->username));
71-
xmlNewChild(newnode, NULL, XMLSTR("password"), XMLSTR(auth_info->password));
70+
xmlNewTextChild(newnode, NULL, XMLSTR("username"), XMLSTR(auth_info->username));
71+
xmlNewTextChild(newnode, NULL, XMLSTR("password"), XMLSTR(auth_info->password));
7272

7373
return AUTH_OK;
7474
}

0 commit comments

Comments
 (0)