Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions POINTER_SAFETY_ANALYSIS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Pointer Access Safety Analysis for PR #14866

## Question
**"is pointer access path safe?"**

Note: The original question asked "is pointer accessath safe?" which we interpret as asking whether pointer access paths (array/index accesses) are safe.

## Answer
**YES - All pointer and index accesses in PR #14866 are SAFE.**

## Summary

This document analyzes the pointer/index access safety in the `rewriteScpLikeGitURL` function introduced in PR #14866 (libutil: remove regex from fixGitURL scp and improve colon handling).

### Function Location
- File: `src/libutil/url.cc`
- Lines: 16-70
- Function: `static std::optional<std::string> rewriteScpLikeGitURL(std::string_view url)`

### All Pointer/Index Accesses Analyzed

#### Access #1: `url[hostStart]` (Line 41)
```cpp
if (url[hostStart] == '[') {
```
- **Bounds Check:** Line 35 checks `if (hostStart >= url.size()) return std::nullopt;`
- **Status:** ✅ SAFE - Only accessed when `hostStart < url.size()`

#### Access #2: `url[close + 1]` (Line 46)
```cpp
if (close + 1 >= url.size() || url[close + 1] != ':')
```
- **Bounds Check:** Uses short-circuit evaluation; `close + 1 >= url.size()` checked BEFORE access
- **Status:** ✅ SAFE - Only accessed when `close + 1 < url.size()`

#### Access #3: `url[colon]` (Line 53)
```cpp
if (colon == std::string_view::npos || url[colon] != ':')
```
- **Bounds Check:** Uses short-circuit evaluation; `colon == npos` checked BEFORE access
- **Status:** ✅ SAFE - Only accessed when colon is a valid index

#### Access #4: `url.substr(colon + 1)` (Line 68)
```cpp
rewritten.append(url.substr(colon + 1));
```
- **Bounds Check:** Line 59 checks `if (colon + 1 >= url.size()) return std::nullopt;`
- **Status:** ✅ SAFE - Only called when `colon + 1 < url.size()`

## Safety Mechanisms Employed

1. **Explicit bounds checking** before direct index access
2. **Short-circuit evaluation** (`||` operator) to prevent unsafe access
3. **Early returns** when invalid conditions detected
4. **Comprehensive test coverage** for edge cases

## Edge Cases Verified

All edge cases are properly handled:
- Empty strings → Returns `std::nullopt`
- Single `@` character → Returns `std::nullopt`
- Missing host/path → Returns `std::nullopt`
- IPv6 brackets → Properly validated
- Unterminated brackets → Returns `std::nullopt`

## Test Coverage

The PR includes extensive tests in `src/libutil-tests/url.cc`:
- `scpLikeEmptyUserDoesNotRewrite`
- `scpLikePathMayContainColon`
- `scpLikeIPv6PathMayContainColon`
- `scpLikeWithSlashBeforeAtDoesNotRewrite`
- Multiple parameterized test cases

## Conclusion

**All pointer accesses in the `rewriteScpLikeGitURL` function are SAFE.**

The code demonstrates excellent defensive programming practices with proper bounds checking before every array access. No security vulnerabilities or undefined behavior detected.

**RECOMMENDATION: The code is safe and ready for merge.**

---
**Analysis performed:** December 26, 2025
**PR:** https://github.com/NixOS/nix/pull/14866
**Commit:** ed0133383510e8bef5a4f0c9ab4cca27a3c8c115