Skip to content
4 changes: 2 additions & 2 deletions server/src/main/java/com/cloud/user/DomainManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ private void updateDomainChildren(DomainVO domain, String updatedDomainPrefix) {
List<DomainVO> domainChildren = _domainDao.findAllChildren(domain.getPath(), domain.getId());
// for each child, update the path
for (DomainVO dom : domainChildren) {
dom.setPath(dom.getPath().replaceFirst(domain.getPath(), updatedDomainPrefix));
dom.setPath(StringUtils.replaceOnce(dom.getPath(), domain.getPath(), updatedDomainPrefix));
_domainDao.update(dom.getId(), dom);
}
}
Expand Down Expand Up @@ -1074,7 +1074,7 @@ protected void validateNewParentDomainCanAccessResourcesOfDomainToBeMoved(String
for (Map.Entry<Long, List<String>> entry : idsOfDomainsWithResourcesUsedByDomainToBeMoved.entrySet()) {
DomainVO domainWithResourceUsedByDomainToBeMoved = _domainDao.findById(entry.getKey());

Pattern pattern = Pattern.compile(domainWithResourceUsedByDomainToBeMoved.getPath().replace("/", "\\/").concat(".*"));
Pattern pattern = Pattern.compile(domainWithResourceUsedByDomainToBeMoved.getPath().replace("/", "\\/").concat(".*")); // This only scaped one Regex metacharacter (/). The wildcard `.` is more common and dangerous in my opinion.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DaanHoogland ,

I did not propose a fix because I need help for that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @DaanHoogland, do not forget this other part of the code that only escapes /, then adds the God of wildcards (.*), and passes for a domain to be moved.

I guess this has the potential of corrupting all resources of the domain being moved or affecting some other unrelated domain that happens to match the Regex.

But I am not qualified to propose a code fix in this case. I would like your help to assess this bug, too.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @sureshanaparti , please note that this is not completed yet. I guess...

Matcher matcher = pattern.matcher(newPathOfDomainToBeMoved);
if (!matcher.matches()) {
domainsOfResourcesInaccessibleToNewParentDomain.put(domainWithResourceUsedByDomainToBeMoved, entry.getValue());
Expand Down
Loading