Skip to content

Commit

Permalink
Try to use a static string for the installer binary
Browse files Browse the repository at this point in the history
  • Loading branch information
@Kidev
Kidev committed Jan 7, 2025
1 parent 9e52aa7 commit 7807a12
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 153 deletions.
197 changes: 56 additions & 141 deletions aqt/installer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1451,18 +1451,10 @@ def download_bin(_base_url):

class CommercialInstaller:

ALLOWED_INSTALLERS = frozenset(
{"qt-unified-windows-x64-online.exe", "qt-unified-macOS-x64-online.dmg", "qt-unified-linux-x64-online.run"}
)

ALLOWED_AUTO_ANSWER_OPTIONS = {
"OperationDoesNotExistError": frozenset({"Abort", "Ignore"}),
"OverwriteTargetDirectory": frozenset({"Yes", "No"}),
"stopProcessesForUpdates": frozenset({"Retry", "Ignore", "Cancel"}),
"installationErrorWithCancel": frozenset({"Retry", "Ignore", "Cancel"}),
"installationErrorWithIgnore": frozenset({"Retry", "Ignore"}),
"AssociateCommonFiletypes": frozenset({"Yes", "No"}),
"telemetry-question": frozenset({"Yes", "No"}),
ALLOWED_INSTALLERS = {
"windows": "qt-unified-windows-x64-online.exe",
"mac": "qt-unified-macOS-x64-online.dmg",
"linux": "qt-unified-linux-x64-online.run",
}

def __init__(

Check failure on line 1460 in aqt/installer.py

View workflow job for this annotation

GitHub Actions / Linter 

[mypy] reported by reviewdog 🐶
error: Function is missing a type annotation for one

Raw Output:
aqt/installer.py:1460:5: error: Function is missing a type annotation for one
Expand Down Expand Up @@ -1511,68 +1503,14 @@ def __init__(
else:
self.os_name = "windows"

self.installer_filename = self._get_installer_filename()
self.qt_account = self._get_qt_account_path()

def _validate_installer_path(self, installer_path: Path) -> None:
"""Validate the installer path is safe and exists"""
try:
# Resolve to absolute path and check for symlink attacks
real_path = installer_path.resolve(strict=True)

if real_path.name not in self.ALLOWED_INSTALLERS:
raise ValueError(f"Invalid installer filename: {real_path.name}")

# Additional file checks
if not real_path.is_file():
raise ValueError("Path is not a regular file")

# On Unix systems, check permissions
if hasattr(os, "access"):
if not os.access(real_path, os.X_OK):
raise ValueError("Installer file is not executable")

except (RuntimeError, OSError) as e:
raise ValueError(f"Invalid installer path: {e}")

@staticmethod
def _validate_dir_path(path: Optional[str]) -> None:
"""Validate a directory path is safe"""
if path:
# Convert to Path object for validation
dir_path = Path(path)
# Ensure path is absolute and normalized
abs_path = dir_path.absolute().resolve()
if not abs_path.parts: # Empty path
raise ValueError("Empty path provided")
if ".." in abs_path.parts: # Path traversal attempt
raise ValueError("Path traversal detected")

@staticmethod
def _validate_auto_answer_option(name: str, value: str, allowed_values: set[str]) -> None:
"""Validate auto-answer option has an allowed value"""
if value not in allowed_values:
raise ValueError(f"Invalid value '{value}' for option {name}")

def _get_installer_filename(self) -> str:
"""Get OS-specific installer filename"""
base = "qt-unified"
self.installer_filename = self.ALLOWED_INSTALLERS[self.os_name]

if self.os_name == "windows":
return f"{base}-windows-x64-online.exe"
self.qt_account = Path(os.environ["APPDATA"]) / "Qt" / "qtaccount.ini"
elif self.os_name == "mac":
return f"{base}-macOS-x64-online.dmg"
self.qt_account = Path.home() / "Library" / "Application Support" / "Qt" / "qtaccount.ini"
else:
return f"{base}-linux-x64-online.run"

def _get_qt_account_path(self) -> Path:
"""Get OS-specific qtaccount.ini path"""
if self.os_name == "windows":
return Path(os.environ["APPDATA"]) / "Qt" / "qtaccount.ini"
elif self.os_name == "mac":
return Path.home() / "Library" / "Application Support" / "Qt" / "qtaccount.ini"
else:
return Path.home() / ".local" / "share" / "Qt" / "qtaccount.ini"
self.qt_account = Path.home() / ".local" / "share" / "Qt" / "qtaccount.ini"

def _download_installer(self, target_path: Path) -> None:
"""Download Qt online installer"""
Expand Down Expand Up @@ -1610,20 +1548,12 @@ def _get_package_name(self) -> str:
qt_version = f"{self.version.major}{self.version.minor}{self.version.patch}"
return f"qt.qt{self.version.major}.{qt_version}.{self.arch}"

def _get_install_command(self, installer_path: Path) -> list[str]:
def _get_install_command(self) -> list[str]:
"""Build installation command with enhanced security validations"""
# Validate installer path first
self._validate_installer_path(installer_path)

# Start with empty command list
cmd: list[str] = []

# Resolve and validate installer path
resolved_installer = str(installer_path.resolve(strict=True))
if not resolved_installer:
raise ValueError("Empty installer path")
cmd.append(resolved_installer)

# Validate credentials if provided
if self.username and self.password:
if not isinstance(self.username, str) or not isinstance(self.password, str):
Expand Down Expand Up @@ -1659,36 +1589,21 @@ def _get_install_command(self, installer_path: Path) -> list[str]:
("AssociateCommonFiletypes", self.associate_common_filetypes),
("telemetry-question", self.telemetry),
]:
# Check against allowed values
allowed_values = self.ALLOWED_AUTO_ANSWER_OPTIONS.get(option)
if not allowed_values or value not in allowed_values:
raise ValueError(f"Invalid value '{value}' for option {option}")
auto_answers.append(f"{option}={value}")

# Add validated auto-answer string
auto_answer_string = ",".join(auto_answers)
if not auto_answer_string:
raise ValueError("Empty auto-answer string")

# Add standard arguments and package
standard_args = [
"--accept-licenses",
"--accept-obligations",
"--confirm-command",
"--auto-answer",
auto_answer_string,
",".join(auto_answers),
"install",
self._get_package_name(),
]

cmd.extend(standard_args)

# Final validation of complete command
if not all(isinstance(arg, str) for arg in cmd):
raise ValueError("Non-string arguments detected")
if not all(arg.strip() for arg in cmd):
raise ValueError("Empty arguments detected")

return cmd

def install(self) -> None:
Expand All @@ -1699,53 +1614,53 @@ def install(self) -> None:
f"or ensure {self.qt_account} exists"
)

with tempfile.TemporaryDirectory(prefix="qt_install_") as temp_dir:
# Create temp dir with appropriate permissions
temp_path = Path(temp_dir)
if hasattr(os, "chmod"):
try:
os.chmod(temp_dir, 0o700) # Restrict to owner-only permissions
except OSError as e:
raise ValueError(f"Failed to set permissions on {temp_dir}: {e}")
# Create temp dir with appropriate permissions
temp_path = Path("/tmp/aqtinstall_qt_online_installer")

Check warning on line 1618 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1618 

Probable insecure usage of temp file/directory.
temp_path.mkdir(exist_ok=True)

installer_path = temp_path / self.installer_filename
installer_path = temp_path / self.installer_filename

self.logger.info(f"Downloading Qt online installer to {installer_path}")
self._download_installer(installer_path)
self.logger.info(f"Downloading Qt online installer to {installer_path}")
self._download_installer(installer_path)

self.logger.info("Starting Qt installation")
self.logger.info("Starting Qt installation")

try:
cmd = self._get_install_command(installer_path)

if not isinstance(cmd, list) or not all(isinstance(arg, str) for arg in cmd):
raise ValueError("Command must be a list of strings")

sanitized_cmd = [arg.strip() for arg in cmd if arg.strip()]
if any(arg.startswith(";") or arg.startswith("&") for arg in sanitized_cmd):
raise ValueError("Invalid command argument detected")

# Log sanitized command (exclude password)
safe_cmd = list(sanitized_cmd)
if "--pw" in safe_cmd:
pw_index = safe_cmd.index("--pw")
if len(safe_cmd) > pw_index + 1:
safe_cmd[pw_index + 1] = "********"
if "--email" in safe_cmd:
email_index = safe_cmd.index("--email")
if len(safe_cmd) > email_index + 1:
safe_cmd[email_index + 1] = "********"
self.logger.info(f"Running: {' '.join(safe_cmd)}")

subprocess.run(safe_cmd, shell=False, check=True, cwd=temp_dir)
except subprocess.CalledProcessError as e:
if e.stderr:
self.logger.error(f": {e.stderr.decode('utf-8', errors='replace')}")
self.logger.error(f"Installation failed with exit code {e.returncode}")
except subprocess.TimeoutExpired:
raise RuntimeError("Installation timed out")
finally:
if installer_path.exists():
installer_path.unlink()

self.logger.info("Qt installation completed successfully")
try:
cmd = self._get_install_command()

# Log sanitized command (exclude password)
safe_cmd = list(cmd)
if "--pw" in safe_cmd:
pw_index = safe_cmd.index("--pw")
if len(safe_cmd) > pw_index + 1:
safe_cmd[pw_index + 1] = "********"
if "--email" in safe_cmd:
email_index = safe_cmd.index("--email")
if len(safe_cmd) > email_index + 1:
safe_cmd[email_index + 1] = "********"
self.logger.info(f"Running: {' '.join(safe_cmd)}")

if self.os_name == "windows":
subprocess.run(

Check failure on line 1644 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1644 

Python possesses many mechanisms to invoke an external executable.
["/tmp/aqtinstall_qt_online_installer/qt-unified-windows-x64-online.exe", *safe_cmd], shell=False

Check warning on line 1645 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1645 

Probable insecure usage of temp file/directory.

Check warning on line 1645 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1645 

subprocess call - check for execution of untrusted input.
)
elif self.os_name == "mac":
subprocess.run(["" + f"{temp_path}/qt-unified-macOS-x64-online.dmg", *safe_cmd], shell=False)

Check failure on line 1648 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1648 

Detected subprocess function 'run' without a static string.

Check failure on line 1648 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1648 

Python possesses many mechanisms to invoke an external executable.

Check warning on line 1648 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1648 

subprocess call - check for execution of untrusted input.
elif self.os_name == "linux":
subprocess.run(
["/tmp/aqtinstall_qt_online_installer/qt-unified-linux-x64-online.run", *safe_cmd], shell=False

Check warning on line 1651 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1651 

Probable insecure usage of temp file/directory.

Check warning on line 1651 in aqt/installer.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

aqt/installer.py#L1651 

subprocess call - check for execution of untrusted input.
)
else:
raise RuntimeError("Unsupported OS")

except subprocess.CalledProcessError as e:
if e.stderr:
self.logger.error(f": {e.stderr.decode('utf-8', errors='replace')}")
self.logger.error(f"Installation failed with exit code {e.returncode}")
except subprocess.TimeoutExpired:
raise RuntimeError("Installation timed out")
finally:
if installer_path.exists():
installer_path.unlink()

self.logger.info("Qt installation completed successfully")
13 changes: 1 addition & 12 deletions tests/test_install.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2060,9 +2060,7 @@ def mock_get_url(url: str, *args, **kwargs) -> str:
"cmd, arch_dict, details, expected_command",
[
(
"install-qt-commercial desktop {} 6.8.0 "
"--outputdir ./install-qt-commercial "
"--user {} --password {}",
"install-qt-commercial desktop {} 6.8.0 " "--outputdir ./install-qt-commercial " "--user {} --password {}",
{"windows": "win64_msvc2022_64", "linux": "linux_gcc_64", "mac": "clang_64"},
["./install-qt-commercial", "qt6", "681"],
"qt-unified-{}-online.run --root {} --accept-licenses --accept-obligations --confirm-command "
Expand All @@ -2085,15 +2083,6 @@ def test_install_qt_commercial(
cli = Cli()
cli._setup_settings()

def mock_exists(*args, **kwargs):
return False

def mock_subprocess(*args, **kwargs):
return 0

monkeypatch.setattr(Path, "exists", mock_exists)
monkeypatch.setattr("subprocess.check_call", mock_subprocess)

cli.run(formatted_cmd.split())

[out, _] = capsys.readouterr()
Expand Down

0 comments on commit 7807a12

Please sign in to comment.