6 Tháng mười hai, 2021
Giới thiệu
Sản phẩm Kiểm đếm nhóm đã yêu cầu chúng tôi xem xét và kiểm toán một bộ hợp đồng với mục tiêu cuối cùng là cải thiện các hợp đồng quản trị chung và cung cấp cho chúng tính linh hoạt hơn. Chúng tôi đã xem mã và bây giờ công bố kết quả của chúng tôi.
Tổng quan về hệ thống
Hệ thống dựa trên ba hợp đồng chính:
- A
SafeGuard
mẫu hợp đồng. Hợp đồng này làadmin
của mộtTimelock
hợp đồng và thêm một lớp vai trò mô-đun trênTimelock
hành động của. Hợp đồng này xác định một số vai trò có trách nhiệm và quyền truy cập riêng biệt đối với trạng thái của một đề xuất (xếp hàng, hủy bỏ và thực hiện). - A
SafeGuardFactory
hợp đồng triển khai một mớiSafeGuard
và một cái mới tương ứngTimelock
hợp đồng. Sau đó, nó đặt địa chỉ timelock bên trongSafeGuard
hợp đồng và đăng ký mớiSafeGuard
vàoRegistry
. - A
Registry
trong đó có một danh sách đã triển khaiSafeGuards
với tương ứng của họ số phiên bản.
Sản phẩm SafeGuardFactory
về cơ bản sẽ sinh ra một mới SafeGuard
bất cứ khi nào nó được gọi. Một SafeGuard
là một cái bọc xung quanh Timelock
hoạt động bổ sung các vai trò khác nhau và riêng biệt cho mỗi hành động. Các vai trò được cấu trúc thông qua việc sử dụng OpenZeppelin's AccessControlEnumerable
hợp đồng mang lại tính linh hoạt và khả năng tương thích cao hơn cho các ví multisig và các trường hợp sử dụng kinh doanh trong đó một số địa chỉ có thể có cùng vai trò chia sẻ.
Cập nhật: Trong tạp chí PR # 10, nhóm Tally đã quyết định xóa Registry
hợp đồng. Danh sách các biện pháp bảo vệ đã triển khai hiện được lưu trữ trong SafeGuardFactory
hợp đồng, trong safeGuards
bộ liệt kê, cùng với phiên bản của chúng được lưu trữ trong safeGuardVersion
lập bản đồ.
Vai trò
Sản phẩm SafeGuard
hợp đồng xác định các vai trò sau:
Cập nhật: Sản phẩm CREATOR_ROLE
vai trò đã bị xóa như một bản sửa lỗi cho sự cố L02.
Phạm vi
Chúng tôi đã kiểm toán cam kết b2c63a9dfc4090be13320d999e7c6c1d842625d3
của safeguard
kho. Trong phạm vi, các hợp đồng thông minh trong contracts
danh mục. Tuy nhiên, mocks
thư mục được coi là nằm ngoài phạm vi.
Giả định
Hệ thống không có nghĩa là có thể nâng cấp được. Các Registry
địa chỉ được thiết lập trong hàm tạo của SafeGuardFactory
và mỗi SafeGuard
có thể có Timelock
chỉ đặt một lần. Điều này có nghĩa là nếu một Registry
được triển khai một cái mới SafeGuardFactory
cũng phải được triển khai. Nếu Timelock
thay đổi triển khai, cũ SafeGuard
s sẽ trở nên lỗi thời và những cái mới sẽ phải được triển khai.
Hơn nữa, hệ thống chủ yếu dựa vào việc triển khai Timelock
hợp đồng được coi là nằm ngoài phạm vi kiểm toán này. Nhóm vẫn chưa hoàn thành việc triển khai Timelock
hợp đồng. Tại thời điểm viết báo cáo này, việc triển khai timelock của Hợp chất đang được sử dụng trong dự án.
Cơ sở mã đã được kiểm tra bởi hai kiểm toán viên trong suốt một tuần và ở đây chúng tôi trình bày những phát hiện của chúng tôi.
Mức độ nghiêm trọng
Không có.
Mức độ nghiêm trọng cao
[H01] ETH có thể bị khóa bên trong Timelock
hợp đồng
Sản phẩm Tally
ban đầu nhóm dựa trên việc triển khai của họ trên cơ sở GovernorBravoDelegate
Hợp đồng ghép.
Trong quá trình đánh giá này, Tally
nhóm phát hiện một hạn chế trong thống đốc của Compound nơi ETH được gửi trực tiếp đến Timelock
không có sẵn để sử dụng bởi các đề xuất quản trị và mặc dù nó không bị mắc kẹt vĩnh viễn, nhưng yêu cầu một giải pháp phức tạp để được truy xuất.
Điều này là do việc thực hiện thống đốc yêu cầu tất cả giá trị của một đề xuất phải được đính kèm như msg.value
bởi tài khoản kích hoạt thực thi, không sử dụng theo bất kỳ cách nào Timelock
Quỹ ETH.
Sản phẩm vấn đề tương tự sau đó đã được xác định trong SafeGuard
thực hiện và nhóm đã biết về vấn đề và đang trong quá trình khắc phục.
Trong khi khắc phục sự cố, hãy cân nhắc sử dụng phương pháp được thông qua bởi thư viện OpenZeppelin cho cùng một vấn đề.
Cập nhật: Đã sửa trong cam kết 7337db227edda83533be586135d96ddac4f5bf29
.
[H02] SafeGuardFactory có thể được đóng băng
Sản phẩm Registry
hợp đồng nhằm theo dõi tất cả các SafeGuards
rằng SafeGuardFactory
sản xuất. Nó có bên ngoài register
chức năng được sử dụng cho mục đích này.
Đồng thời, SafeGuardFactory
có, trong hàm tạo của nó, việc gán địa phương registry
giá trị thành giá trị đầu vào. Không có khả năng thay đổi giá trị của registry
và vì lý do này, nếu một Registry
được triển khai, một nhà máy mới cũng phải được triển khai.
Sản phẩm SafeGuardFactory
có createSafeGuard
chức năng, phụ trách đầu tiên triển khai một cái mới SafeGuard
thì mới Timelock
với địa chỉ của SafeGuard
as admin
, sau đó thiết lập timelock
biến của SafeGuard
hợp đồng và cuối cùng đăng ký SafeGuard
trong sổ đăng ký.
Vấn đề là bất kỳ cuộc gọi nào đến createSafeGuard
có thể bị buộc phải thất bại bởi kẻ tấn công có thể đăng ký trực tiếp địa chỉ xác định của SafeGuard
trước khi tạo ra nó. Bất cứ khi nào một hợp đồng tạo ra một new
ví dụ, nonce của nó được tăng lên và địa chỉ của nơi mà phiên bản mới của hợp đồng sẽ được triển khai có thể được xác định bởi địa chỉ hợp đồng ban đầu và nonce của nó. Do đó, kẻ tấn công có thể tính toán trước nhiều địa chỉ mà SafeGuards
sẽ được triển khai và đăng ký các địa chỉ đó trong Registry
bằng cách gọi register
hàm số. Điều này sẽ dẫn đến các cuộc gọi đến createSafeGuard
đến trở lại kể từ khi Registry
đã chứa địa chỉ.
Để tránh các tác nhân bên ngoài gọi công khai Register
hợp đồng, hãy xem xét việc hạn chế quyền truy cập vào register
chức năng chấp nhận cuộc gọi độc quyền bởi SafeGuardFactory
.
Cập nhật: Cố định PR # 10. Nhóm Tally đã xóa Registry
hợp đồng.
Mức độ nghiêm trọng trung bình
Không có.
Mức độ nghiêm trọng thấp
[L01] Đã nhận xét ra mã
Sản phẩm Registry
hợp đồng bao gồm một dòng mã đã nhận xét. Để cải thiện khả năng đọc, hãy xem xét xóa nó khỏi codebase.
Cập nhật: Cố định PR # 10 và cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L02] Quản trị viên của SafeGuard có thể chỉ định vai trò của người tạo cho bất kỳ địa chỉ nào
Sản phẩm SafeGuard
hợp đồng xác định vai trò của một CREATOR_ROLE
mà, như tên cho thấy, được chỉ định cho người tạo ra biện pháp bảo vệ.
Tuy nhiên, bằng cách gọi các grantRole
chức năng của AccessControlEnumerable
hợp đồng trong thư viện hợp đồng OpenZeppelin, quản trị viên có thể cấp vai trò này cho bất kỳ địa chỉ nào. Điều này có thể gây nhầm lẫn vì người tạo ra SafeGuard
chỉ có thể là SafeGuardFactory
.
Trong toàn bộ cơ sở mã, vai trò này chỉ được sử dụng để hạn chế người dùng tương tác với setTimelock
chức năng của SafeGuard
hợp đồng. Theo thiết kế, hệ thống đảm bảo rằng setTimelock
chức năng chỉ có thể được gọi một lần, Từ trong SafeGuardFactory
hợp đồng.
Xem xét việc loại bỏ CREATOR_ROLE
vai trò từ SafeGuard
hợp đồng và sử dụng onlyOwner
thay đổi trong setTimelock
chức năng.
Cập nhật: Cố định PR # 10.
[L03] Định nghĩa và triển khai giao diện không chính xác
Sản phẩm ISafeGuard
giao diện không xác định queueTransactionWithDescription
chức năng được thực hiện trong SafeGuard
hợp đồng, đồng thời, nó xác định __abdicate, __queueSetTimelockPendingAdmin và __executeSetTimelockPendingAdmin nhưng chúng không được thực hiện.
Để cải thiện tính đúng đắn và nhất quán trong cơ sở mã, hãy xem xét cấu trúc lại ISafeGuard
giao diện khớp chính xác SafeGuard
thực hiện.
Cập nhật: Đã sửa trong cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
[L04] Thiếu chuỗi tài liệu
Một số hợp đồng và chức năng trong cơ sở mã thiếu tài liệu. Ví dụ, một số chức năng trong SafeGuard
hợp đồng.
Ngoài ra, một số docstrings sử dụng ngôn ngữ không chính thức, chẳng hạn như phía trên setTimelock
chức năng trong SafeGuard
hợp đồng.
Điều này cản trở sự hiểu biết của người đánh giá về mục đích của mã, điều này là cơ bản để đánh giá một cách chính xác không chỉ tính bảo mật mà còn cả tính đúng đắn. Ngoài ra, docstrings cải thiện khả năng đọc và dễ bảo trì. Họ phải giải thích rõ ràng mục đích hoặc ý định của các chức năng, các tình huống mà chúng có thể thất bại, các vai trò được phép gọi chúng, các giá trị được trả về và các sự kiện được phát ra.
Cân nhắc kỹ lưỡng việc ghi lại tất cả các chức năng (và các tham số của chúng) là một phần của API công khai của hợp đồng. Các chức năng triển khai chức năng nhạy cảm, ngay cả khi không công khai, cũng phải được ghi lại rõ ràng. Khi viết docstrings, hãy xem xét việc làm theo Định dạng đặc điểm kỹ thuật tự nhiên của Ethereum (NaSpec).
Cập nhật: Đã sửa một phần trong PR # 10. Các docstrings thích hợp đã được thêm vào các chức năng khác nhau trong toàn bộ cơ sở mã. Tuy nhiên, ngoài những thay đổi hiện tại, hãy xem xét thực hiện những thay đổi sau:
- Thêm
description
như@param
trong docstring ở trênqueueTransactionWithDescription
chức năng - Thêm
@param
trong chuỗi tài liệu phía trêncreateSafeGuard
chức năng trongSafeGuardFactory
hợp đồng - Thêm
@return
trong docstrings phía trên các hàm trongSafeGuardFactory
hợp đồng.
[L05] Mã lặp lại hoặc vô dụng
Có những nơi trong cơ sở mã nơi mã được lặp lại hoặc không cần thiết. Một số ví dụ:
- Dòng 29-32 của
Registry
hợp đồng là vô ích, bởi vì_add
chức năng củaEnumerableSet
hợp đồng đã thực hiện những kiểm tra này chống lại các giá trị đã được đặt. - Dòng 62, 67, 73 và 78 của
SafeGuard
hợp đồng đều lặp lại cùng một hoạt động chính xác. Cân nhắc đóng gói nó vào một hàm bên trong để tránh trùng lặp mã. - Dòng 62-63 và 67-68 of
SafeGuard
được lặp lại. Hãy xem xét gói gọn chúng vào một chức năng nội bộ duy nhất. - Việc sử dụng
gasleft
để chỉ định bao nhiêu khí nên được chuyển tiếp trong lệnh gọi của hàmexecuteTransaction
là không cần thiết. Điều này là do, tại thời điểm thực hiện, toàn bộ khí còn lại sẽ được sử dụng để tiếp tục thực hiện. Nếu điều này không rõ ràng, hãy xem xét xóagas
tham số từ cuộc gọi.
Cân nhắc áp dụng bản sửa được đề xuất để tạo mã rõ ràng hơn và cải thiện tính nhất quán và tính mô-đun trên cơ sở mã.
Cập nhật: Cố định PR # 10 và cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
Ghi chú & Thông tin bổ sung
[N01] Phong cách không nhất quán
Có một số chỗ trong cơ sở mã, nơi mà sự khác biệt về kiểu dáng ảnh hưởng đến khả năng đọc, khiến việc hiểu mã trở nên khó khăn hơn. Một số ví dụ:
- Sản phẩm
Registry
hợp đồng sử dụng các kiểu khác nhau cho docstrings trong toàn bộ hợp đồng. - Sản phẩm
SafeGuard
hợp đồng đang tạo ra một sự kiện khiqueueTransactionWithDescription
được gọi nhưng không có sự kiện nào được phát ra trong các hàm khác xử lý các giao dịch. - Trong tạp chí
SafeGuard
hợp đồng, đôi khi giá trị được sử dụng làm tham số được đặt tên và đôi khi _giá trị Được sử dụng.
Cân nhắc giá trị mà một phong cách mã hóa nhất quán làm tăng khả năng đọc của dự án, hãy cân nhắc việc thực thi một phong cách mã hóa tiêu chuẩn với sự trợ giúp của các công cụ linter, chẳng hạn như Solhint.
Cập nhật: Cố định PR # 10 và cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N02] Thiếu giấy phép
Các hợp đồng sau trong cơ sở mã thiếu số nhận dạng giấy phép SPDX.
Để tắt tiếng cảnh báo của trình biên dịch và tăng tính nhất quán trên toàn bộ codebase, hãy xem xét thêm số nhận dạng giấy phép. Trong khi làm điều đó, hãy xem xét đề cập đến spdx.dev hướng dẫn.
Cập nhật: Cố định PR # 10 và cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N03] Sự phụ thuộc của Hợp đồng OpenZeppelin không được ghim
Để ngăn chặn các hành vi không mong muốn trong trường hợp các thay đổi vi phạm được phát hành trong các bản cập nhật trong tương lai của Thư viện OpenZeppelin Contracts ', hãy xem xét ghim phiên bản của phụ thuộc này trong package.json tập tin.
Cập nhật: Cố định PR # 10.
[N04] Phiên bản trình biên dịch Solidity không được ghim
Trong suốt cơ sở mã, hãy xem xét ghim phiên bản của Trình biên dịch Solidity lên phiên bản ổn định mới nhất của nó. Điều này sẽ giúp ngăn chặn việc giới thiệu các lỗi không mong muốn do các bản phát hành trong tương lai không tương thích. Để chọn một phiên bản cụ thể, các nhà phát triển nên xem xét cả các tính năng của trình biên dịch mà dự án cần và danh sách các lỗi đã biết liên kết với mỗi phiên bản trình biên dịch Solidity.
Cập nhật: Cố định PR # 10.
[N05] Đánh máy
Tại các trường hợp khác nhau trong toàn bộ cơ sở mã, từ role
viết sai chính tả là rol
. Một ví dụ như vậy là trong chuỗi tài liệu trong constructor
của SafeGuard
hợp đồng.
Xem xét sửa những lỗi chính tả này để cải thiện khả năng đọc mã.
Cập nhật: Đã sửa một phần trong PR # 10. Trong khi chính tả của role
đã được sửa, nhận xét "đặt vai trò quản trị viên thành địa chỉ quản trị xác định" phải là "đặt vai trò quản trị viên thành địa chỉ quản trị viên xác định". Ngoài ra, "thực thi" bị sai chính tả trong SafeGuard
hợp đồng trên dòng 69, dòng 82, dòng 96 và dòng 110 và "có sẵn" bị viết sai chính tả trên dòng 70, dòng 83, dòng 97, dòng 111. Ngoài ra, hãy xem xét thay thế các từ không chính thức như "sẽ" in SafeGuard
ký hợp đồng với các lựa chọn thay thế chính thức chẳng hạn như “sẽ đến”.
[N06] Khai báo uint là uint256
Có một số lần xuất hiện trong cơ sở mã nơi các biến được khai báo uint
kiểu dữ liệu thay vì uint256
. Ví dụ: eta
biến trong QueueTransactionWithDescription
sự kiện của SafeGuard
hợp đồng.
Để ủng hộ sự rõ ràng, tất cả các trường hợp của uint
nên được khai báo là uint256
.
Cập nhật: Cố định PR # 10 và cam kết 7fd27df16fc879d990d36a167a0b6e719e578558
.
[N07] Nhập chưa sử dụng
Sản phẩm SafeGuard
hợp đồng nhập khẩu console
hợp đồng nhưng không bao giờ sử dụng nó.
Để cải thiện khả năng đọc của mã, hãy xem xét loại bỏ bất kỳ dữ liệu nhập không sử dụng nào.
Cập nhật: Cố định PR # 10.
Kết luận
Một lỗ hổng cao và một số lỗ hổng nhỏ khác đã được tìm thấy và các đề xuất cũng như các bản sửa lỗi đã được đề xuất.
- &
- truy cập
- Tài khoản
- ngang qua
- Hoạt động
- hành động
- thêm vào
- địa chỉ
- quản trị viên
- Tất cả
- Đã
- Mặc dù
- api
- phương pháp tiếp cận
- xung quanh
- kiểm toán
- Về cơ bản
- được
- lỗi
- kinh doanh
- cuộc gọi
- trường hợp
- Nguyên nhân
- thay đổi
- phí
- mã
- Lập trình
- Chung
- Hợp chất
- nhầm lẫn
- xem xét
- chứa
- tiếp tục
- hợp đồng
- hợp đồng
- có thể
- yaratıcı
- Current
- dữ liệu
- xử lý
- Thiết kế
- phát triển
- khác nhau
- phát hiện
- Kỹ lưỡng
- ETH
- Sự kiện
- sự kiện
- ví dụ
- nhà máy
- Tính năng
- Cuối cùng
- Tên
- Sửa chữa
- Linh hoạt
- tìm thấy
- chức năng
- quỹ
- tương lai
- GAS
- Cho
- mục tiêu
- quản trị
- Thống đốc
- hướng dẫn
- có
- giúp đỡ
- tại đây
- Cao
- Độ đáng tin của
- HTTPS
- thực hiện
- Tăng lên
- tăng
- Giao thức
- IT
- nổi tiếng
- Ngôn ngữ
- mới nhất
- Thư viện
- Giấy phép
- Dòng
- Danh sách
- địa phương
- khóa
- nhìn
- Làm
- Trận đấu
- mô-đun
- Đa cấp
- Nền tảng khác
- trình bày
- quá trình
- dự án
- đề nghị
- công khai
- xuất bản
- ghi danh
- Phát hành
- báo cáo
- kho
- Kết quả
- xem xét
- an ninh
- định
- thiết lập
- chia sẻ
- thông minh
- Hợp đồng thông minh
- sự vững chắc
- Tiểu bang
- phong cách
- hệ thống
- Thông qua
- khắp
- thời gian
- công cụ
- theo dõi
- Giao dịch
- Cập nhật
- us
- Người sử dụng
- giá trị
- Lỗ hổng
- Ví
- tuần
- CHÚNG TÔI LÀ
- ở trong
- từ
- viết