Skip to content

Commit f3ebfa3

Browse files
NMS-13125: Escape userId & groupId
1 parent 101e3aa commit f3ebfa3

File tree

10 files changed

+43
-32
lines changed

10 files changed

+43
-32
lines changed

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/groupDetail.jsp

+4-3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
%>
4141

4242
<%@page import="org.opennms.web.group.WebGroup"%>
43+
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
4344

4445
<%
4546
WebGroup group = (WebGroup)request.getAttribute("group");
@@ -60,13 +61,13 @@
6061
<div class="col-md-6">
6162
<div class="card">
6263
<div class="card-header">
63-
<span>Details for Group: <%=group.getName()%></span>
64+
<span>Details for Group: <%=WebSecurityUtils.sanitizeString(group.getName())%></span>
6465
</div>
6566
<table class="table table-sm">
6667
<tr>
6768
<th>Comments:</th>
6869
<td width="75%">
69-
<%=group.getComments()%>
70+
<%=WebSecurityUtils.sanitizeString(group.getComments())%>
7071
</td>
7172
</tr>
7273
<tr>
@@ -79,7 +80,7 @@
7980
<% } else { %>
8081
<ul class="list-unstyled">
8182
<% for (String user : users) { %>
82-
<li> <%=user%> </li>
83+
<li> <%=WebSecurityUtils.sanitizeString(user)%> </li>
8384
<% } %>
8485
</ul>
8586
<% } %>

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/list.jsp

+10-9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
--%>
3131

3232
<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
33+
<%@taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %>
3334

3435
<jsp:include page="/includes/bootstrap.jsp" flush="false" >
3536
<jsp:param name="title" value="Group Configuration" />
@@ -116,37 +117,37 @@
116117
<th>Comments</th>
117118
</tr>
118119
<c:forEach var="group" varStatus="groupStatus" items="${groups}">
119-
<tr class="divider ${groupStatus.index % 2 == 0 ? 'even' : 'odd'}" id="group-${group.name}">
120+
<tr class="divider ${groupStatus.index % 2 == 0 ? 'even' : 'odd'}" id="group-${fn:escapeXml(group.name)}">
120121
<td width="5%" class="text-center">
121122
<c:choose>
122-
<c:when test='${group.name != "Admin"}'>
123-
<a id="${group.name}.doDelete" href="javascript:deleteGroup('${group.name}')" onclick="return confirm('Are you sure you want to delete the group ${group.name}?')"><i class="fa fa-trash-o fa-2x"></i></a>
123+
<c:when test='${fn:escapeXml(group.name) != "Admin"}'>
124+
<a id="${group.name}.doDelete" href="javascript:deleteGroup('${fn:escapeXml(group.name)}')" onclick="return confirm('Are you sure you want to delete the group ${fn:escapeXml(group.name)}?')"><i class="fa fa-trash-o fa-2x"></i></a>
124125
</c:when>
125126
<c:otherwise>
126-
<i class="fa fa-trash-o fa-2x" onclick="alert('Sorry, the ${group.name} group cannot be deleted.')"></i>
127+
<i class="fa fa-trash-o fa-2x" onclick="alert('Sorry, the ${fn:escapeXml(group.name)} group cannot be deleted.')"></i>
127128
</c:otherwise>
128129
</c:choose>
129130
</td>
130131
<td width="5%" class="text-center">
131-
<a id="${group.name}.doModify" href="javascript:modifyGroup('${group.name}')"><i class="fa fa-edit fa-2x"></i></a>
132+
<a id="${fn:escapeXml(group.name)}.doModify" href="javascript:modifyGroup('${fn:escapeXml(group.name)}')"><i class="fa fa-edit fa-2x"></i></a>
132133
</td>
133134
<td width="5%" class="text-center">
134135
<c:choose>
135136
<c:when test='${group.name != "Admin"}'>
136-
<button id="${group.name}.doRename" type="button" class="btn btn-secondary" name="rename" onclick="renameGroup('${group.name}')">Rename</button>
137+
<button id="${fn:escapeXml(group.name)}.doRename" type="button" class="btn btn-secondary" name="rename" onclick="renameGroup('${fn:escapeXml(group.name)}')">Rename</button>
137138
</c:when>
138139
<c:otherwise>
139-
<button id="${group.name}.doRename" type="button" class="btn btn-secondary" name="rename" onclick="alert('Sorry, the Admin group cannot be renamed.')">Rename</button>
140+
<button id="${fn:escapeXml(group.name)}.doRename" type="button" class="btn btn-secondary" name="rename" onclick="alert('Sorry, the Admin group cannot be renamed.')">Rename</button>
140141
</c:otherwise>
141142
</c:choose>
142143
</td>
143144
<td>
144-
<a href="javascript:detailGroup('${group.name}')">${group.name}</a>
145+
<a href="javascript:detailGroup('${fn:escapeXml(group.name)}')">${fn:escapeXml(group.name)}</a>
145146
</td>
146147
<td>
147148
<c:choose>
148149
<c:when test="${group.comments.isPresent()}">
149-
${group.comments.get()}
150+
${fn:escapeXml(group.comments.get())}
150151
</c:when>
151152

152153
<c:otherwise>

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/admin/userGroupView/groups/modifyGroup.jsp

+2-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
"
4040
%>
4141
<%@page import="org.opennms.web.group.WebGroup"%>
42+
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
4243

4344
<%
4445
WebGroup group = (WebGroup)session.getAttribute("group.modifyGroup.jsp");
@@ -463,7 +464,7 @@
463464
private String createSelectList(String name, String[] categories) {
464465
StringBuffer buffer = new StringBuffer("<select class=\"form-control custom-select\" multiple=\"multiple\" name=\""+name+"\" size=\"10\">");
465466
for(String category : categories){
466-
buffer.append("<option>" + category + "</option>");
467+
buffer.append("<option>" + WebSecurityUtils.sanitizeString(category) + "</option>");
467468
}
468469
buffer.append("</select>");
469470

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/alarm/detail.jsp

+1-1
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@
449449
</tr>
450450
<% for (OnmsAcknowledgment ack : acks) {%>
451451
<tr class="severity-<%=alarm.getSeverity().getLabel().toLowerCase()%>">
452-
<td><%=ack.getAckUser()%></td>
452+
<td><%=WebSecurityUtils.sanitizeString(ack.getAckUser())%></td>
453453
<td><%=ack.getAckAction()%></td>
454454
<td><onms:datetime date="<%=ack.getAckTime()%>" /></td>
455455
</tr>

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/event/detail.jsp

+1-1
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@
107107
<th class="col-1">Node</th>
108108
<td ${acknowledgeEvent ? '' : 'colspan="3"'} class="${acknowledgeEvent ? 'col-3' : 'col-7'}">
109109
<% if( event.getNodeId() > 0 ) { %>
110-
<a href="element/node.jsp?node=<%=event.getNodeId()%>"><%=event.getNodeLabel()%></a>
110+
<a href="element/node.jsp?node=<%=event.getNodeId()%>"><%=WebSecurityUtils.sanitizeString(event.getNodeLabel())%></a>
111111
<% } else {%>
112112
&nbsp;
113113
<% } %>

Diff for: opennms-webapp/src/main/webapp/WEB-INF/jsp/outage/detail.jsp

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
<th class="col-2">Node</th>
7373
<td class="col-2">
7474
<% if( outage.getNodeId() > 0 ) { %>
75-
<a href="element/node.jsp?node=<%=outage.getNodeId()%>"><%=outage.getNodeLabel()%></a>
75+
<a href="element/node.jsp?node=<%=outage.getNodeId()%>"><%=WebSecurityUtils.sanitizeString(outage.getNodeLabel())%></a>
7676
<% } else {%>
7777
&nbsp;
7878
<% } %>

Diff for: opennms-webapp/src/main/webapp/admin/userGroupView/users/list.jsp

+16-11
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@
3636
<%@page import="java.util.*" %>
3737
<%@page import="org.opennms.netmgt.config.*" %>
3838
<%@page import="org.opennms.netmgt.config.users.*" %>
39+
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
40+
41+
<%@taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %>
42+
3943
<%
4044
UserManager userFactory;
4145
Map<String,User> users = null;
@@ -97,7 +101,7 @@
97101
var newID = prompt("Enter new name for user.", userID);
98102
99103
if (newID != null && newID != "") {
100-
if (/.*[&<>"`']+.*/.test(newId)) {
104+
if (/.*[&<>"`']+.*/.test(newID)) {
101105
alert("The user ID must not contain any HTML markup.");
102106
return;
103107
}
@@ -154,54 +158,55 @@
154158
String textService = userFactory.getTextPage(userid);
155159
String numericPin = userFactory.getNumericPin(userid);
156160
String textPin = userFactory.getTextPin(userid);
161+
String sanitizedUserId = WebSecurityUtils.sanitizeString(curUser.getUserId());
157162
%>
158163
<tr id="user-<%= userid %>">
159164
<% if (!curUser.getUserId().equals("admin") && !curUser.getUserId().equals("rtc")) { %>
160165
<td rowspan="2" class="text-center">
161-
<a id="<%= "users("+curUser.getUserId()+").doDelete" %>" href="javascript:deleteUser('<%=curUser.getUserId()%>')" onclick="return confirm('Are you sure you want to delete the user <%=curUser.getUserId()%>?')"><i class="fa fa-trash-o fa-2x"></i></a>
166+
<a id="<%= "users("+sanitizedUserId+").doDelete" %>" href="javascript:deleteUser('<%=sanitizedUserId%>')" onclick="return confirm('Are you sure you want to delete the user <%=sanitizedUserId%>?')"><i class="fa fa-trash-o fa-2x"></i></a>
162167
</td>
163168
<% } else { %>
164169
<td rowspan="2" class="text-center">
165170
<i class="fa fa-trash-o fa-2x" onclick="alert('Sorry, the admin user cannot be deleted.')"></i>
166171
</td>
167172
<% } %>
168173
<td rowspan="2" class="text-center">
169-
<a id="<%= "users("+curUser.getUserId()+").doModify" %>" href="javascript:modifyUser('<%=curUser.getUserId()%>')"><i class="fa fa-edit fa-2x"></i></a>
174+
<a id="<%= "users("+sanitizedUserId+").doModify" %>" href="javascript:modifyUser('<%=sanitizedUserId%>')"><i class="fa fa-edit fa-2x"></i></a>
170175
</td>
171176
<td rowspan="2" class="text-center">
172177
<% if ( !curUser.getUserId().equals("admin")) { %>
173-
<button id="<%= "users("+curUser.getUserId()+").doRename" %>" class="btn btn-secondary" name="rename" onclick="renameUser('<%=curUser.getUserId()%>')">Rename</button>
178+
<button id="<%= "users("+sanitizedUserId+").doRename" %>" class="btn btn-secondary" name="rename" onclick="renameUser('<%=sanitizedUserId%>')">Rename</button>
174179
<% } else { %>
175-
<button id="<%= "users("+curUser.getUserId()+").doRename" %>" class="btn btn-secondary" name="rename" onclick="alert('Sorry, the admin user cannot be renamed.')">Rename</button>
180+
<button id="<%= "users("+sanitizedUserId+").doRename" %>" class="btn btn-secondary" name="rename" onclick="alert('Sorry, the admin user cannot be renamed.')">Rename</button>
176181
<% } %>
177182
</td>
178183
<td>
179-
<a id="<%= "users("+curUser.getUserId()+").doDetails" %>" href="javascript:detailUser('<%=curUser.getUserId()%>')"><%=curUser.getUserId()%></a>
184+
<a id="<%= "users("+sanitizedUserId+").doDetails" %>" href="javascript:detailUser('<%=sanitizedUserId%>')"><%=sanitizedUserId%></a>
180185
</td>
181186
<td>
182-
<div id="<%= "users("+curUser.getUserId()+").fullName" %>">
187+
<div id="<%= "users("+sanitizedUserId+").fullName" %>">
183188
<%= (curUser.getFullName().orElse("")) %>
184189
</div>
185190
</td>
186191
<td>
187-
<div id="<%= "users("+curUser.getUserId()+").email" %>">
192+
<div id="<%= "users("+sanitizedUserId+").email" %>">
188193
<%= ((email == null || email.equals("")) ? "&nbsp;" : email) %>
189194
</div>
190195
</td>
191196
<td>
192-
<div id="<%= "users("+curUser.getUserId()+").pagerEmail" %>">
197+
<div id="<%= "users("+sanitizedUserId+").pagerEmail" %>">
193198
<%= ((pagerEmail == null || pagerEmail.equals("")) ? "&nbsp;" : pagerEmail) %>
194199
</div>
195200
</td>
196201
<td>
197-
<div id="<%= "users("+curUser.getUserId()+").xmppAddress" %>">
202+
<div id="<%= "users("+sanitizedUserId+").xmppAddress" %>">
198203
<%= ((xmppAddress == null || xmppAddress.equals("")) ? "&nbsp;" : xmppAddress) %>
199204
</div>
200205
</td>
201206
</tr>
202207
<tr>
203208
<td colspan="5">
204-
<div id="<%= "users("+curUser.getUserId()+").userComments" %>">
209+
<div id="<%= "users("+sanitizedUserId+").userComments" %>">
205210
<%= (curUser.getUserComments().orElse("No Comments")) %>
206211
</div>
207212
</td>

Diff for: opennms-webapp/src/main/webapp/admin/userGroupView/users/modifyUser.jsp

+3-2
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
<%@page import="org.opennms.netmgt.config.users.*"%>
4141
<%@page import="org.opennms.web.api.Util"%>
4242
<%@page import="org.opennms.web.api.Authentication"%>
43+
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
4344
<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c"%>
4445

4546
<%!
@@ -253,15 +254,15 @@
253254
</script>
254255

255256
<form role="form" class="form-horizontal" id="modifyUser" method="post" name="modifyUser">
256-
<input id="userID" type="hidden" name="userID" value="<%=user.getUserId()%>"/>
257+
<input id="userID" type="hidden" name="userID" value="<%=WebSecurityUtils.sanitizeString(user.getUserId())%>"/>
257258
<input id="password" type="hidden" name="password"/>
258259
<input id="redirect" type="hidden" name="redirect"/>
259260

260261
<div class="row">
261262
<div class="col-md-6">
262263
<div class="card">
263264
<div class="card-header">
264-
<span>Modify User: <%=userid%></span>
265+
<span>Modify User: <%=WebSecurityUtils.sanitizeString(userid)%></span>
265266
</div>
266267
<div class="card-body">
267268
<h3>User Password</h3>

Diff for: opennms-webapp/src/main/webapp/admin/userGroupView/users/userDetail.jsp

+3-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
org.opennms.web.servlet.MissingParameterException
4040
"
4141
%>
42+
<%@ page import="org.opennms.core.utils.WebSecurityUtils" %>
43+
<%@ page import="java.util.stream.Collectors" %>
4244

4345
<%
4446
User user = null;
@@ -72,7 +74,7 @@
7274
<div class="col-md-6">
7375
<div class="card">
7476
<div class="card-header">
75-
<span>Details for User: <%=user.getUserId()%></span>
77+
<span>Details for User: <%=WebSecurityUtils.sanitizeString(user.getUserId())%></span>
7678
</div>
7779
<table class="table table-sm">
7880
<tr>

Diff for: opennms-webapp/src/main/webapp/notification/detail.jsp

+2-2
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@
114114
</c:choose>
115115
</td>
116116
<th class="col-md-1">Responder</th>
117-
<td class="col-md-2"><%=notice.getResponder()!=null ? notice.getResponder() : "&nbsp;"%></td>
117+
<td class="col-md-2"><%=notice.getResponder()!=null ? WebSecurityUtils.sanitizeString(notice.getResponder()) : "&nbsp;"%></td>
118118
<th class="col-md-1">Location</th>
119119
<td class="col-md-2">
120120
<c:choose>
@@ -237,7 +237,7 @@
237237
<% for (NoticeSentTo sentTo : notice.getSentTo()) { %>
238238

239239
<tr class="severity-<%=eventSeverity.toLowerCase()%>">
240-
<td><%=sentTo.getUserId()%></td>
240+
<td><%=WebSecurityUtils.sanitizeString(sentTo.getUserId())%></td>
241241

242242
<td>
243243
<c:choose>

0 commit comments

Comments
 (0)