Skip to content

Commit

Permalink
util: security: critical: use full hash length
Browse files Browse the repository at this point in the history
Historically those hashes were shortened to result in shorter URLs and
firmware image names. This however turns out to be a security issue
allowing malicious cache injections. Use the full length now.

Ref: GHSA-r3gq-96h6-3v7q

Reported-by: @Ry0taK
Signed-off-by: Paul Spooren <[email protected]>
Signed-off-by: Petr Štetiar <[email protected]> [commit subject, test fix]
  • Loading branch information
aparcar authored and ynezz committed Dec 6, 2024
1 parent deadda8 commit d4c9e8b
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 17 deletions.
2 changes: 1 addition & 1 deletion asu/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def build(build_request: BuildRequest, job=None):
"image",
f"PROFILE={build_request.profile}",
f"PACKAGES={' '.join(build_cmd_packages)}",
f"EXTRA_IMAGE_NAME={packages_hash}",
f"EXTRA_IMAGE_NAME={packages_hash[:12]}",
f"BIN_DIR=/builder/{request_hash}",
]

Expand Down
11 changes: 3 additions & 8 deletions asu/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
from asu.config import settings


REQUEST_HASH_LENGTH: int = 32

log: logging.Logger = logging.getLogger("rq.worker")
log.propagate = False # Suppress duplicate log messages.

Expand Down Expand Up @@ -66,18 +64,16 @@ def get_branch(version_or_branch: str) -> dict[str, str]:
return {**settings.branches.get(branch_name, {}), "name": branch_name}


def get_str_hash(string: str, length: int = REQUEST_HASH_LENGTH) -> str:
def get_str_hash(string: str) -> str:
"""Return sha256sum of str with optional length
Args:
string (str): input string
length (int): hash length
Returns:
str: hash of string with specified length
"""
h = hashlib.sha256(bytes(string or "", "utf-8"))
return h.hexdigest()[:length]
return hashlib.sha256(bytes(string or "", "utf-8")).hexdigest()


def get_file_hash(path: str) -> str:
Expand Down Expand Up @@ -145,7 +141,6 @@ def get_request_hash(build_request: BuildRequest) -> str:
str(build_request.repositories),
]
),
REQUEST_HASH_LENGTH,
)


Expand All @@ -161,7 +156,7 @@ def get_packages_hash(packages: list[str]) -> str:
Returns:
str: hash of `req`
"""
return get_str_hash(" ".join(sorted(list(set(packages)))), 12)
return get_str_hash(" ".join(sorted(list(set(packages)))))


def fingerprint_pubkey_usign(pubkey: str) -> str:
Expand Down
9 changes: 6 additions & 3 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ def test_api_build_request_hash(client):
profile="testprofile",
)

case12hash = "8731372f84b0022c070e6127bad24eb2"
case34hash = "0dab3b60bd8174da250e2ea2942a3744"
case12hash = "8d8e0aa2fd95bb75dba4aff4279dd6f976a40ad17300927d54b8a9a9b0576306"
case34hash = "6b1645013216da39ee09deae75b87b0636f3c50648b037750b0a80448ce5c7ca"

# Case 1 - diff_packages=True, first package ordering
json["diff_packages"] = True
Expand Down Expand Up @@ -702,7 +702,10 @@ def test_api_build_defaults_filled_allowed(app):

assert response.status_code == 200
data = response.json()
assert data["request_hash"] == "c9836b2259eec0c3a31868fe77a19983"
assert (
data["request_hash"]
== "9c8d0cd7d9ec208a233b954edb20c3c20b5c11103bb7f5f1ebface565f8c6720"
)


def test_api_build_defaults_filled_too_big(app):
Expand Down
22 changes: 17 additions & 5 deletions tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,47 @@


def test_get_str_hash():
assert get_str_hash("test", 12) == "9f86d081884c"
assert (
get_str_hash("test")
== "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"
)


def test_get_file_hash():
file_fd, file_path = tempfile.mkstemp()
os.write(file_fd, b"test")

assert get_file_hash(file_path).startswith("9f86d081884c")
assert (
get_file_hash(file_path)
== "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"
)

os.close(file_fd)
os.unlink(file_path)


def test_get_packages_hash():
assert get_packages_hash(["test1", "test2"]) == "57aab5949a36"
assert (
get_packages_hash(["test1", "test2"])
== "57aab5949a36e66b535a8cb13e39e9e093181c9000c016990d7be9eb86a9b9e8"
)


def test_get_request_hash():
request = BuildRequest(
**{
"distro": "test",
"version": "test",
"target": "test",
"target": "testtarget/testsubtarget",
"profile": "test",
"packages": ["test"],
}
)

assert get_request_hash(request) == "3944eba49da93e2c605a7e9980e52765"
assert (
get_request_hash(request)
== "99ff721439cd696f7da259541a07d7bfc7eb6c45a844db532e0384b464e23f46"
)


def test_diff_packages():
Expand Down

0 comments on commit d4c9e8b

Please sign in to comment.