forked from yencarnacion/html4tree
-
Notifications
You must be signed in to change notification settings - Fork 0
repair: remove non-portable 4096 pathname security limit #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
12
commits into
master
Choose a base branch
from
sentinel-path-length-limit-15858591483294204787
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
3167e2c
Add max length validation for topDir to prevent DoS vulnerabilities
seonghobae 3631476
Add max length validation for topDir to prevent DoS vulnerabilities
seonghobae 4320b3b
Merge branch 'master' into sentinel-path-length-limit-158585914832942…
opencode-agent[bot] a30b1c6
test(path): reject arbitrary 4096 portability limit RED
seonghobae 044f68d
fix(path): defer pathname limits to filesystem semantics
seonghobae 596b8c6
docs(gap): correct pathname limit threat model
seonghobae cfaf669
docs(security): remove superseded path-limit claim
seonghobae e80d070
chore(path): restore exact protected implementation after false-posit…
seonghobae 00dd17e
test(path): remove obsolete arbitrary-length assertion
seonghobae 8d8669a
Merge branch 'master' into sentinel-path-length-limit-158585914832942…
opencode-agent[bot] b18b1de
Add max length validation for topDir to prevent DoS vulnerabilities
seonghobae 38f5acd
repair(path): restore verified portable validation tree
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Product / Technical Gap Baseline | ||
|
|
||
| 이 문서는 html4tree의 live code와 열린 PR을 기준으로 상용화 Gap을 추적한다. 병합되지 않은 branch의 동작을 protected `master`의 현재 기능으로 간주하지 않는다. | ||
|
|
||
| ## Pathname portability and resource claims | ||
|
|
||
| ### 문제 | ||
|
|
||
| PR #598은 `topDir.length <= 4096`을 보안 경계로 추가하고, 4,097자 경로가 OOM/DoS를 일으킬 수 있다는 HIGH severity 설명을 붙였다. 이 설명과 숫자는 portable filesystem contract가 아니다. | ||
|
|
||
| Java `File`은 pathname string ↔ abstract pathname 변환이 본질적으로 system-dependent라고 명시한다. POSIX 계열은 filesystem/path-resolution 단계에서 `NAME_MAX`/`PATH_MAX`와 `ENAMETOOLONG`을 다루며, Windows는 전통적인 `MAX_PATH` 260 제한과 long-path opt-in 이후의 다른 동작을 함께 가진다. 따라서 Kotlin `String.length`의 UTF-16 code-unit 수 4,096을 모든 지원 환경의 경로 한계처럼 적용하면 실제 filesystem semantics와 분리된 임의 제한이 된다. | ||
|
|
||
| 또한 CLI가 이미 `String` 인자를 구성한 뒤 `go()`에 전달하므로 4,096자 검사 자체가 argument allocation OOM을 예방하는 경계도 아니다. 4,097자 문자열은 그 자체로 HIGH severity resource-exhaustion evidence가 아니다. | ||
|
|
||
| ### 선택 | ||
|
|
||
| - `go()`는 임의의 4,096 UTF-16 code-unit 상한을 사용하지 않는다. | ||
| - 경로의 실제 존재 여부와 symlink/root 정책은 Java/NIO와 underlying filesystem semantics를 통해 검증한다. | ||
| - 경로 길이만으로 HIGH/OOM 취약점을 주장하지 않는다. | ||
| - 실제 crawl resource exhaustion은 입력 문자열 길이와 분리해 디렉토리 수, entry fan-out, 출력량, I/O 시간/메모리 profile을 기준으로 다뤄야 한다. 이 PR에서 검증되지 않은 arbitrary crawl budget을 새로 정하지 않는다. | ||
|
|
||
| ### RED → GREEN evidence | ||
|
|
||
| - RED `a30b1c60d8036546a43140a6c63dd41d56b7c851`: 4,097자 문자열이라는 이유만으로 portable filesystem limit 오류를 반환하지 않고, 기존 filesystem validity contract로 실패해야 한다는 회귀 테스트를 추가했다. 당시 branch의 4,096 검사 때문에 이 계약은 실패한다. | ||
| - GREEN `044f68d25b9aa0d7bcde3da16c4d81b4c701337c`: 임의 4,096 length gate를 제거했다. 기존 blank/traversal/root/symlink/directory 검증은 유지한다. | ||
|
|
||
| ### 남은 Gap | ||
|
|
||
| `maxLevel=-1`은 깊이 제한이 없으며 실제 대규모 tree에서 디렉토리 수·fan-out·생성 파일 수에 따른 자원 사용량을 별도로 측정하지 않는다. 이를 DoS로 분류하거나 새 기본 상한을 넣기 전에 real/right-cleared representative trees에서 directory count, peak RSS, filesystem calls, elapsed time, generated bytes를 측정하고 buyer contract를 정해야 한다. | ||
|
|
||
| ### References | ||
|
|
||
| - Oracle. (2023). *File (Java SE 21 & JDK 21)*. Java Platform, Standard Edition documentation. https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/io/File.html | ||
| - Microsoft. (n.d.). *Maximum Path Length Limitation*. Windows App Development documentation. https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation | ||
| - The Open Group / IEEE. (n.d.). *pathconf()* / pathname limit semantics. POSIX interfaces as published in Linux man-pages. https://man7.org/linux/man-pages/man3/fpathconf.3p.html | ||
|
|
||
| 위 자료는 pathname semantics가 OS/filesystem에 종속된다는 기술 계약을 확인하는 primary documentation으로 사용한다. 특정 filesystem의 수치를 다른 플랫폼의 universal application limit로 전환하지 않는다. |
19 changes: 19 additions & 0 deletions
19
src/test/kotlin/html4tree/PathValidationPortabilityTest.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package html4tree | ||
|
|
||
| import org.junit.Test | ||
| import kotlin.test.assertEquals | ||
| import kotlin.test.assertFailsWith | ||
|
|
||
| class PathValidationPortabilityTest { | ||
| @Test | ||
| fun pathLengthIsNotTreatedAsAPortableFilesystemLimit() { | ||
| val error = assertFailsWith<IllegalArgumentException> { | ||
| go("a".repeat(4097), -1) | ||
| } | ||
|
|
||
| assertEquals( | ||
| "Top directory must be an existing non-symlink directory", | ||
| error.message | ||
| ) | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔍 Documentation changes trigger full CI
Removing both
paths-ignorefilters runs the full build for documentation-only changes. Confirm this added CI cost is required.Was this helpful? React with 👍 or 👎 to provide feedback.