[GEOS-11274] Fix JSON Legend with external reference #7362
Conversation
| // other dirs are not published, serialize uri-data | ||
| Path path = file.toPath(); | ||
| String contentType = Files.probeContentType(path); | ||
| byte[] bytes = Files.readAllBytes(path); | ||
| return new StringBuilder("data:") | ||
| .append(contentType) | ||
| .append(";base64,") | ||
| .append(Base64.getEncoder().encodeToString(bytes)) | ||
| .toString(); |
There was a problem hiding this comment.
Relative references should work too though, the contents of the styles directory is available to the users.
So for example, the "grass_poly.sld" file in the release data directory contains this relative reference:
<OnlineResource xlink:type="simple" xlink:href="grass_fill.png" />
which is accessible using a link to the origin GeoServer, e.g.:
https://gs-main.geosolutionsgroup.com/geoserver/styles/grass_fill.png
Long story short, it seems to me the "file.isAbsolute" test is too strict
There was a problem hiding this comment.
This fix is about the contents of the styles directory for specific workspaces. These resources are not published. Links to /geoserver/sf/styles/ExternalGraphicIcon.png do not work.
But you are correct in what you say and the test I added is probably misleading you. I have included a new test that covers the same code but it is clearer. If you prefer, I can remove the other test.
There was a problem hiding this comment.
Still problematic though... what's preventing someone from sending a dynamic SLD style, in SLD_BODY, with a relative reference that will cause GeoServer to embed in the JSON response the contents of any random file on the file system?
Better to keep just a URL, and make the contents of workspace styles published just like the main styles instead, the location is controlled in this case. That would require modifying the StylePublisher and its mappings, making sure it cannot be used to access other folders.
@sikeoka thoughts on this matter?
| // other dirs are not published, serialize uri-data | ||
| Path path = file.toPath(); | ||
| String contentType = Files.probeContentType(path); | ||
| byte[] bytes = Files.readAllBytes(path); | ||
| return new StringBuilder("data:") | ||
| .append(contentType) | ||
| .append(";base64,") | ||
| .append(Base64.getEncoder().encodeToString(bytes)) | ||
| .toString(); |
There was a problem hiding this comment.
Still problematic though... what's preventing someone from sending a dynamic SLD style, in SLD_BODY, with a relative reference that will cause GeoServer to embed in the JSON response the contents of any random file on the file system?
Better to keep just a URL, and make the contents of workspace styles published just like the main styles instead, the location is controlled in this case. That would require modifying the StylePublisher and its mappings, making sure it cannot be used to access other folders.
@sikeoka thoughts on this matter?
|
Very good point. This is indeed a serious security breach. At the very least, I should have verified that the resource is in the styles directory, just as it is checked for the main styles directory. This condition should be verified as a security check if the resource is serialized, or as a simple precaution if an URL is going to be generated. On the other hand, I just realized that the contents of the styles directory for specific workspaces ARE published. However, the URL is not what I expected. For example, geoserver/styles/sf/ExternalGraphicIcon.png does work. I am going to push some changes with this in mind. |
|
I did not notice your last change. Could you update the PR and align it with the main branch? (there are a couple of conflicts) |
|
Done. Also fixed the test. |
| @@ -0,0 +1,28 @@ | |||
| <StyledLayerDescriptor xmlns="http://www.opengis.net/sld" xmlns:ogc="http://www.opengis.net/ogc" xmlns:xlink="http://www.w3.org/1999/xlink" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | |||
There was a problem hiding this comment.
While toppSample below is used, I don't see this new file being used anywhere
This a a PR for GEOS-11274
It prevents NullPointerException providing links in the form of URI Data for unresolvable urls and excluding
urlandexternal-graphic-urlwhen not possible.Checklist
mainbranch (backports managed later; ignore for branch specific issues).For core and extension modules:
[GEOS-XYZWV] Title of the Jira ticket.