rzo1 opened a new issue, #2081:
URL: https://github.com/apache/stormcrawler/issues/2081
## What happens
`crawler-default.yaml` ships `protocols: "http,https,file"` and maps
`file.protocol.implementation` to `FileProtocol`, so the file handler is live
in every topology that starts from the library defaults. `FileProtocol` has no
notion of a permitted directory: `FileResponse` takes the path out of the URL,
decodes it and reads whatever the worker user can read, and a directory is
turned into a synthetic sitemap with `isSitemap=true`, which
`SiteMapParserBolt` expands into further `file://` URLs. The only thing that
keeps `file://` links out of the crawl is URL filtering, and the library
default ships no URL filters at all (`urlfilters.config.file` is commented out
at crawler-default.yaml:243). Absolute `file://` hrefs found in a fetched page
resolve to themselves in the parser and reach the status store like any other
outlink.
## Where
`core/src/main/resources/crawler-default.yaml:199` and `:202`, config keys
`protocols` and `file.protocol.implementation`.
```yaml
protocols: "http,https,file"
file.protocol.implementation:
"org.apache.stormcrawler.protocol.file.FileProtocol"
```
`core/src/main/java/org/apache/stormcrawler/protocol/file/FileResponse.java:62`:
```java
File file = new File(URLDecoder.decode(path,
fileProtocol.getEncoding()));
```
`FileProtocol.getRobotRules` (FileProtocol.java:44) returns `EMPTY_RULES`,
so nothing else gates the read.
## Why it matters
A crawled page can put a `file://` URL into the frontier, and on a topology
built on library defaults the fetcher will read that path and index the bytes.
Files readable by the worker user include the topology configuration itself, so
backend credentials can end up in the search index. Projects generated from the
archetype are not affected as shipped: `default-regex-filters.txt:2` contains
`-^(file|ftp|mailto):`, so the exposure needs a project that does not use that
filter file or that has edited it. Enabling the file scheme deliberately, for a
local corpus, still exposes the whole readable filesystem, because there is no
way to confine reads to a directory.
## Reproduction
Save as
`core/src/test/java/org/apache/stormcrawler/protocol/file/FileProtocolDefaultsTest.java`.
```java
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to you under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.stormcrawler.protocol.file;
import java.io.File;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import org.apache.storm.Config;
import org.apache.storm.utils.Utils;
import org.apache.stormcrawler.Metadata;
import org.apache.stormcrawler.protocol.ProtocolResponse;
import org.apache.stormcrawler.util.ConfUtils;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
class FileProtocolDefaultsTest {
/** The file scheme must not be part of the shipped default protocols
list. */
@Test
void fileSchemeIsNotEnabledByDefault() {
Config conf = new Config();
Map<String, Object> defaults =
Utils.findAndReadConfigFile("crawler-default.yaml", false);
conf.putAll(ConfUtils.extractConfigElement(defaults));
String protocols = ConfUtils.getString(conf, "protocols",
"http,https");
List<String> schemes = Arrays.asList(protocols.split(" *, *"));
Assertions.assertFalse(
schemes.contains("file"),
"crawler-default.yaml enables the file scheme by default: "
+ protocols);
}
/**
* With a root directory configured, FileProtocol must refuse to read a
file outside that root.
*/
@Test
void fileProtocolConfinesReadsToConfiguredRoot(@TempDir Path tmp) throws
Exception {
Path base = tmp.toRealPath();
Path root = base.resolve("root");
Files.createDirectories(root);
Path outside = base.resolve("outside.txt");
Files.write(outside, "not for the
crawler".getBytes(StandardCharsets.UTF_8));
Config conf = new Config();
conf.put("file.protocol.root", root.toString());
FileProtocol protocol = new FileProtocol();
protocol.configure(conf);
String url = new File(outside.toString()).toURI().toURL().toString();
ProtocolResponse response = protocol.getProtocolOutput(url, new
Metadata());
Assertions.assertNotEquals(
200,
response.getStatusCode(),
"FileProtocol read a file outside the configured root: " +
url);
}
}
```
Run it:
```
mvn -pl core test -Dtest=FileProtocolDefaultsTest
```
Both cases assert the wanted behaviour and fail on main.
```
[ERROR] FileProtocolDefaultsTest.fileSchemeIsNotEnabledByDefault:46
crawler-default.yaml enables the file scheme by default: http,https,file ==>
expected: <false> but was: <true>
[ERROR]
FileProtocolDefaultsTest.fileProtocolConfinesReadsToConfiguredRoot:71
FileProtocol read a file outside the configured root:
file:/private/var/folders/.../junit-.../outside.txt ==> expected: not equal but
was: <200>
[ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0
```
The second case configures `file.protocol.root` and reads a file outside it.
The key does not exist yet, so the read succeeds with status 200.
## Suggested fix
Change `crawler-default.yaml:199` to `protocols: "http,https"` so the file
handler has to be switched on deliberately. Add a root directory option to
`FileProtocol.configure`, for example `file.protocol.root`, and check it in the
`FileResponse` constructor after canonicalisation: a path outside the root
returns 403 rather than content. Decide what an unset root means; refusing to
serve anything until a root is set is the safer reading, and either way the
behaviour needs a release note, since topologies that crawl local corpora on
the current default will stop working until they set the two keys.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
