Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-8fp4-rp6c-5gcv
Motivation:

- We changed how `PathAndQuery` handles `%2F` (/) in 1.12.0 via #3855.
  This change introduces an unexpected hole in its double-dot detection
  logic.
- Since we decided not to decode `%2F`, we should not decode it
  whereever possible.

Modifications:

- Hardened the double-dot detection logic in `PathAndQuery`.
- `Bytes.data` now always store the bytes in their decoded form. We keep
  whether the byte has to be encoded in a separate `BitSet`.
- Split `ArmeriaHttpUtil.decodePath()` into `decodePath()` and
  `decodePathParam()`.
  - We don't decode `%2F` in `decodePath()` but we do in
    `decodePathParam()`.
  - `RoutingResultBuilder.rawParam()` now uses `decodePathParam()`
    because `decodePath()` doesn't decode `%2F` anymore.

Result:

- A path that contains double dots with `%2F`, such as
  `/files/..%2Fsecrets.txt`, are now rejected correctly.
  • Loading branch information
trustin committed Dec 2, 2021
1 parent a110551 commit e2697a5
Show file tree
Hide file tree
Showing 6 changed files with 501 additions and 222 deletions.
Expand Up @@ -304,11 +304,37 @@ public static String concatPaths(@Nullable String path1, @Nullable String path2)
*/
public static String decodePath(String path) {
if (path.indexOf('%') < 0) {
// No need to decoded; not percent-encoded
// No need to decode because it's not percent-encoded
return path;
}

// Decode percent-encoded characters, but don't decode %2F into /, so that a user can choose
// to use it as a non-separator.
//
// For example, for the path pattern `/orgs/{org_name}/agents/{agent_name}`:
// - orgs/mi6/agents/ethan-hunt
// - org_name: mi6
// - agent_name: ethan-hunt
// - orgs/mi%2F6/agents/ethan-hunt
// - org_name: mi/6
// - agent_name: ethan-hunt
return slowDecodePath(path, false);
}

/**
* Decodes a single percent-encoded path parameter.
*/
public static String decodePathParam(String pathParam) {
if (pathParam.indexOf('%') < 0) {
// No need to decode because it's not percent-encoded
return pathParam;
}

// Decode percent-encoded characters.
return slowDecodePath(pathParam, true);
}

private static String slowDecodePath(String path, boolean decodeSlash) {
// An invalid character is replaced with 0xFF, which will be replaced into '�' by UTF-8 decoder.
final int len = path.length();
try (TemporaryThreadLocals tempThreadLocals = TemporaryThreadLocals.acquire()) {
Expand All @@ -335,7 +361,14 @@ public static String decodePath(String path) {
// The first or second digit is not hexadecimal.
buf[dstLen++] = (byte) 0xFF;
} else {
buf[dstLen++] = (byte) ((digit1 << 4) | digit2);
final byte decoded = (byte) ((digit1 << 4) | digit2);
if (decodeSlash || decoded != 0x2F) {
buf[dstLen++] = decoded;
} else {
buf[dstLen++] = '%';
buf[dstLen++] = '2';
buf[dstLen++] = (byte) path.charAt(i); // f or F - preserve the case.
}
}
}

Expand Down

0 comments on commit e2697a5

Please sign in to comment.