Skip to content

Add structural validation layer and harden install scripts#8

Merged
chigichan24 merged 13 commits into
mainfrom
consider-security-problem
Mar 21, 2026
Merged

Add structural validation layer and harden install scripts#8
chigichan24 merged 13 commits into
mainfrom
consider-security-problem

Conversation

@chigichan24
Copy link
Copy Markdown
Owner

Summary

  • 構造検証レイヤー追加: magic bytes 判定後に各画像フォーマットのヘッダ構造を検証し、不正なデータのクリップボード配置を防止。デフォルトON、--no-validate でスキップ可能
  • install.sh セキュリティ強化: SHA256 チェックサム検証、パスバリデーション、trap によるクリーンアップ、VERSION フォーマット検証
  • release スクリプト強化: バージョン文字列の semver バリデーション、sed injection 修正、checksums.txt 生成、permissions 最小化
  • 制御文字フィルタ: UTF-8/Latin-1 両方のテキストから C0 制御文字と DEL を除去し、ターミナル escape sequence injection を防止

Changes

Structural validation (8 validators)

Format Checks
PNG IHDR chunk present, width/height > 0
JPEG Valid marker after SOI (0xC0–0xFE)
GIF Logical Screen Descriptor width/height > 0
TIFF IFD offset ≥ 8 and < data size
BMP DIB header size is known valid value
WebP VP8/VP8L/VP8X chunk header present
HEIC/AVIF ftyp box size ≥ 12 per ISO 14496-12
PDF Rejects /JS, /JavaScript, /OpenAction, /AA, /Launch with boundary-aware matching

Install/release hardening

  • install.sh: checksum verification, absolute path + .. rejection, trap cleanup EXIT, VERSION semver check
  • release-artifactbundle.sh: version format validation, sed delimiter |
  • release.yml: shasum -a 256 checksums generation, explicit permissions

Other

  • DataType conforms to CustomStringConvertible for stable error messages
  • ByteReader as Data extension with boundary-checked Optional returns
  • Control character stripping via allSatisfy on all Unicode scalars
  • README security section with "Important limitations" subsection
  • Architecture docs updated to reflect all changes

Test plan

  • swift test — 74 tests pass (24 detection + 50 validation)
  • cat valid.png | xpbc → image copied to clipboard
  • Malformed PNG (missing IHDR) piped → exit 1 with descriptive error
  • --no-validate with malformed data → validation skipped, clipboard written
  • Verify install.sh rejects relative paths and .. paths

🤖 Generated with Claude Code

chigichan24 and others added 13 commits March 22, 2026 00:31
Foundation for the structural validation layer that checks image
header integrity beyond magic byte detection.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Structural validation layer that checks image header integrity beyond
magic byte detection. Each validator performs lightweight checks:

- PNG: IHDR chunk presence, width/height > 0
- JPEG: valid marker after SOI (0xC0-0xFE range)
- GIF: Logical Screen Descriptor width/height > 0
- TIFF: IFD offset within valid range
- BMP: DIB header size matches known valid values
- WebP: VP8/VP8L/VP8X chunk header presence
- HEIC/AVIF: ftyp box size sanity check
- PDF: rejects files with /JS, /JavaScript, /OpenAction, /AA, /Launch

Also adds validationFailed case to XPBCError.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers all 8 format validators plus text passthrough:
- PNG: valid, too short, missing IHDR, zero width/height
- JPEG: valid, no marker prefix, invalid range, too short
- GIF: valid, zero width/height, too short
- TIFF: valid LE/BE, offset too small/exceeds data, too short
- BMP: valid DIB sizes (40, 124), invalid size, too short
- WebP: VP8/VP8L/VP8X valid, unknown chunk, too short
- HEIC/AVIF: valid, box size too small/exceeds data
- PDF: valid, /JS, /JavaScript, /OpenAction, /AA, /Launch
- Text: always valid

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Validation runs by default after format detection. If the image header
is malformed, xpbc exits with error code 1 and a descriptive message.
Use --no-validate to skip validation for trusted inputs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds Important limitations subsection to Security section explaining
that structural validation cannot detect crafted exploit payloads,
decompression bombs, or obfuscated PDF active content. Warns against
piping untrusted data.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate INSTALL_DIR is an absolute path and does not contain '..'
- Download checksums.txt from release and verify SHA256 before extraction
- Use curl -fsSL to fail on HTTP errors
- Send error messages to stderr

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Validate VERSION_STRING matches semver format vX.Y.Z
- Change sed delimiter from '/' to '|' to prevent injection via version string
- Send error messages to stderr

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Generate SHA256 checksums.txt and upload as release asset
- Explicitly set packages, issues, pull-requests permissions to none

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ape injection

When decodeText falls back to Latin-1 encoding, strip C0 control characters
(except tab, newline, carriage return) and DEL (0x7F) to prevent potential
terminal escape sequence injection from untrusted input.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical:
- ByteReader: Convert to Data extension with Optional return and
  boundary checks to prevent out-of-bounds crashes

Warning:
- DataValidator: Replace Dictionary lookup with exhaustive switch
  so compiler catches missing validators when DataType grows
- PDFValidator: Add boundary-aware keyword matching to reduce false
  positives (/JSActions, /AABattery no longer trigger), fail-close
  on decode failure
- DataType: Add CustomStringConvertible for stable error messages
- PasteboardWriter: Apply control character filter to UTF-8 text
  too (not just Latin-1 fallback), extract stripControlCharacters
- install.sh: Add trap cleanup EXIT for intermediate file cleanup

Suggestion:
- FtypValidator: Add ISO BMFF comment for boxSize 0/1, explicit
  Int cast for UInt32/Int comparison
- Tests: Add boundary value tests (PNG 29/28 bytes, ftyp boxSize==8),
  PDF false positive tests, total 69 test cases

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Warning fixes:
- stripControlCharacters: Use allSatisfy over all Unicode scalars
  instead of first! to handle multi-scalar graphemes safely
- PDFValidator: Add comment documenting hex escape bypass limitation
- FtypValidator: Raise minimum ftyp box size from 8 to 12 per
  ISO 14496-12 (header 8 + major brand 4)
- install.sh: Add VERSION format validation (semver pattern)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add ftyp boundary value test for boxSize=11 (just below minimum 12)
- Make stripControlCharacters internal for testability
- Add stripControlCharacters tests: ESC removal, tab/newline/CR
  preservation, NUL removal, multi-scalar grapheme passthrough
- Total: 74 test cases

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add DataValidator and FormatValidator to data flow diagram
- Document all 8 format validators and their specific checks
- Document safe byte-reading Data extension (boundary-checked, Optional)
- Document PDF boundary-aware keyword matching and hex escape limitation
- Document control character stripping for terminal injection prevention
- Update error handling table with validationFailed case
- Update module boundaries with new public/internal types
- Update "Adding a New Format" guide (now 7 steps including validator)
- Update testing section: 74 tests across 2 suites with detailed breakdown

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@chigichan24 chigichan24 merged commit 0f8a171 into main Mar 21, 2026
1 check passed
@chigichan24 chigichan24 deleted the consider-security-problem branch March 21, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant