December 16, 2021
บทนำ
พื้นที่ 1inch ทีมขอให้เราตรวจสอบและตรวจสอบของพวกเขา โปรโตคอลจำกัดการสั่งซื้อ สัญญาอัจฉริยะ v2 เราดูโค้ดแล้วเผยแพร่ผลลัพธ์ของเรา
ขอบเขต
เราตรวจสอบคำมั่นสัญญา 4d94eea25e4dac6271bfd703096a5c4a4d899b4a
ของ 1inch/limit-order-protocol
ที่เก็บ ในขอบเขตมีสัญญาดังต่อไปนี้:
- OrderMixin.sol
- OrderRFQMixin.sol
- PredicateHelper.sol
- RevertReasonParser.sol
- Permitable.sol
- ChainlinkCalculator.sol
- ArgumentsDecoder.sol
- AmountCalculator.sol
- NonceManager.sol
- LimitOrderProtocol.sol
- ImmutableOwner.sol
- InteractiveNotificationReceiver.sol
- AggregatorInterface.sol
- IDaiLikePermit.sol
ไฟล์โปรเจ็กต์และไดเร็กทอรีอื่นๆ ทั้งหมด (รวมถึงการทดสอบ) พร้อมกับการพึ่งพาภายนอกและโปรเจ็กต์ ทฤษฎีเกม และการออกแบบสิ่งจูงใจ ก็ไม่รวมอยู่ในขอบเขตของการตรวจสอบนี้ด้วย รหัสภายนอกและการพึ่งพาสัญญาได้รับการสันนิษฐานว่าทำงานเป็นเอกสารและบริการแบ็คเอนด์ที่มีให้โดย 1inch ถือว่าทำหน้าที่เพื่อประโยชน์สูงสุดของโปรโตคอล
สุขภาพโดยรวม
โดยทั่วไป เราพบว่า codebase ของโปรเจ็กต์สามารถอ่านได้และมีการจัดระเบียบที่ดี แม้ว่าจะได้ประโยชน์จากเอกสารประกอบที่ครอบคลุมมากขึ้น โดยเฉพาะอย่างยิ่งรอบๆ บล็อคของโค้ดแอสเซมบลี, Edge case ของโปรโตคอล, แอสเซท/เพรดิเคต/ทรัพยากรภายนอกที่จะถูกใช้งาน ความรับผิดชอบ/ข้อจำกัดของบริการแบ็คเอนด์ที่มีให้ และการโต้ตอบระหว่างนักแสดง โปรเจ็กต์นี้ใช้ความพยายามอย่างมากในการทำให้การดำเนินการเป็นไปอย่างมีประสิทธิภาพ แม้ในบางครั้งอาจมีความเสี่ยงที่จะทำให้โค้ดยากขึ้นในการให้เหตุผล เราแจ้งปัญหาที่เกี่ยวข้องด้านล่าง ตลอดการตรวจสอบ ทีมงาน 1 นิ้วมีความพร้อมใช้งานสูง ตอบสนอง และใช้งานง่ายมาก
ภาพรวมของระบบ
Limit Order Protocol เปิดใช้งานคำสั่งซื้อ makers
เพื่อลงนามคำสั่งนอกเครือข่ายสำหรับการแลกเปลี่ยนโทเค็น โปรโตคอลจะอำนวยความสะดวกในการกรอกคำสั่งซื้อที่ลงนามก่อนหน้านี้ตามคำสั่ง takers
. คำสั่งซื้อสามารถขยายได้สูง และสามารถเรียกสัญญาภายนอกแบบคงที่ได้หลายจุดตลอดกระบวนการกรอกคำสั่งซื้อ การขยายนี้ทำให้โปรโตคอลมีอรรถประโยชน์ แต่เพิ่มความซับซ้อนและพื้นผิวการโจมตีที่มากขึ้นสำหรับคำสั่งด้วยตัวมันเอง
สิ่งสำคัญคือต้องสังเกตว่าไม่มีการจัดเก็บรายละเอียดคำสั่งซื้อในสายโซ่ สถานะการเติมหรือสถานะการยกเลิกของคำสั่งซื้อจะถูกติดตามผ่านแฮชของคำสั่งซื้อเท่านั้น สิ่งนี้จำเป็นที่คำสั่งต้องแชร์แบบเพียร์ทูเพียร์หรือผ่านฝ่ายที่รวมศูนย์ ในกรณีนี้ ทีม 1inch ตั้งใจที่จะทำหน้าที่เป็นฝ่ายรวมศูนย์นั้น รวบรวมคำสั่งซื้อที่ลงนามแล้วและใช้คำสั่งเหล่านั้นเป็นแหล่งของสภาพคล่องสำหรับโปรโตคอลอื่นๆ คำสั่งซื้อจะเผยแพร่ผ่าน API ของตนเอง เพื่อให้ผู้ใช้สามารถโต้ตอบกับคำสั่งซื้อได้
การรวมศูนย์นี้ช่วยให้ทีมขนาด 1 นิ้วสามารถควบคุมคำสั่งซื้อที่เผยแพร่และดำเนินการได้ในที่สุด นอกจากนี้ยังช่วยให้พวกเขาสามารถเซ็นเซอร์คำสั่งซื้อ ซึ่งอาจเป็นประโยชน์ในกรณีของคำสั่งที่เป็นอันตรายหรือหลอกลวง แต่ยังอาจถูกนำไปใช้ในทางที่ผิดและอนุญาตให้พวกเขาดำเนินการกับผู้ใช้รายอื่นในกรณีที่มีคำสั่งซื้อที่ดีโดยไม่แสดงผ่าน API
บทบาทที่ได้รับสิทธิพิเศษ
แม้ว่าสัญญาที่ใช้บทบาทจะไม่อยู่ในขอบเขต แต่ก็มีการระบุบทบาทที่มีสิทธิพิเศษไว้หนึ่งบทบาท หนึ่ง immutableOwner
ถูกกำหนดให้เป็นผู้สร้างสัญญามอบฉันทะในขณะก่อสร้าง และใช้เพื่อจำกัดการเข้าถึงของผู้รับมอบฉันทะ external
ฟังก์ชั่น
การพึ่งพาภายนอกและสมมติฐานความน่าเชื่อถือ
การออกแบบโปรโตคอลนี้จำเป็นต้องมีส่วนประกอบแบบ off-chain และ on-chain และโมเดลไฮบริดนี้สามารถใช้เพื่อบรรเทาเวกเตอร์การโจมตีที่เราระบุในรายงานของเรา แต่ต้นทุนของความสามารถนั้นเพิ่มขึ้นโดยอาศัยทีมขนาด 1 นิ้วและโครงสร้างพื้นฐาน
นอกจากนี้ Limit Order Protocol ยังมีฟังก์ชันที่มุ่งดึงราคาจาก Chainlink oracles เราถือว่าออราเคิลเหล่านั้นมีความซื่อสัตย์ เข้าถึงได้ และทำงานอย่างถูกต้อง
นอกจากนี้ เนื่องจากความยืดหยุ่นของคำสั่งซื้อ มีจุดติดต่อกับสัญญาภายนอกหลายแห่งที่ไม่ผ่านการตรวจสอบ ซึ่งหมายความว่าผู้ใช้ที่ประสงค์ร้ายสามารถใช้การเรียกดังกล่าวในทางที่ผิดและปลอมแปลงภาคแสดง สินทรัพย์ หรือ oracles ด้วยสัญญาที่เป็นอันตรายเพื่อดำเนินการในระหว่างการกรอกคำสั่งซื้อ แม้ว่าโครงการจะได้รับการคุ้มครองในบางพื้นที่จากการกลับเข้ามาใหม่ แต่พาหะดังกล่าวอาจทำให้เกิดการปฏิเสธการโจมตีบริการหรือคำสั่งสแปมที่ตรวจไม่พบ ทีมงาน 1inch ทราบดีว่าปัญหาบางอย่างอาจปรากฏขึ้นเมื่อใช้สัญญาที่ไม่คุ้นเคยสำหรับโปรโตคอลและได้ระบุเจตนาของพวกเขาที่จะสนับสนุนเฉพาะสินทรัพย์ "blue Chip" ที่สำคัญเท่านั้นโดยโครงการ อย่างไรก็ตาม ควรสังเกตว่าแม้กับสินทรัพย์ที่ได้รับความนิยมมากที่สุด ก็ยังมีพฤติกรรมที่แท้จริงจากแต่ละสินทรัพย์ที่อาจทำให้เกิดปัญหากับโปรโตคอลที่ไม่สามารถจัดการได้อย่างถูกต้อง เช่น มีค่าธรรมเนียมระหว่างการโอนด้วย USDT หรือการส่งคืนรหัสข้อผิดพลาดแทน บูลีนที่ประสบความสำเร็จด้วย cTokens
ผลการวิจัย
ที่นี่เรานำเสนอผลการวิจัยของเรา
ความรุนแรงที่สำคัญ
ไม่
ความรุนแรงสูง
[H01] ข้อมูลที่ไม่สอดคล้องกันถูกส่งผ่านไปยัง _makeCall
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร OrderMixin
สัญญา _makeCall
ทำหน้าที่โอนทรัพย์สิน จากผู้รับสู่ผู้สร้าง แล้วก็ จากผู้สร้างสู่ผู้รับ. ในการโอนครั้งหลัง _makeCall
ฟังก์ชันส่งผ่านคำสั่งของ .อย่างไม่ถูกต้อง makerAsset
เป็นพารามิเตอร์สุดท้ายเมื่อควรเป็นคำสั่งของ makerAssetData
.
ด้วยเหตุนี้ ฟังก์ชันพร็อกซีใดๆ ที่อาศัย makerAssetData
อาร์กิวเมนต์จะแตก
เพื่อให้สอดคล้องกับการเรียกครั้งก่อนถึง _makeCall
และเพื่อสนับสนุนฟังก์ชันพร็อกซี่อย่างเต็มที่ ให้พิจารณาอัปเดต order.makerAsset
พารามิเตอร์ order.makerAssetData
.
ปรับปรุง: แก้ไขใน ดึงคำขอ #57.
[H02] คำสั่งซื้อส่วนตัวที่กรอกบางส่วนสามารถกรอกได้โดยทุกคน
โปรโตคอลอนุญาตให้สร้างคำสั่งส่วนตัวและสาธารณะ ในการสั่งซื้อส่วนตัวเท่านั้น allowedSender
ที่อยู่ที่ระบุโดยผู้ผลิตในระหว่างการสร้างคำสั่งซื้อ สามารถกรอกคำสั่งซื้อได้
อย่างไรก็ตามใน OrderMixin
สัญญา, การตรวจสอบสำหรับ allowedSender
ที่อยู่ มีการกำหนดขอบเขตอย่างไม่ถูกต้อง หมายความว่ามีการประเมินภายในตรรกะที่จัดการการเติมคำสั่งซื้อครั้งแรกเท่านั้น หากคำสั่งซื้อส่วนตัวเต็มไปบางส่วนแล้วให้ตรวจสอบ allowedSender
ไม่สามารถเข้าถึงได้อีกต่อไปและทุกคนสามารถกรอกคำสั่งซื้อได้
เพื่อชี้แจงเจตนาว่าผู้ใช้รายใดควรจะสามารถกรอกคำสั่งซื้อส่วนตัวที่กรอกข้อมูลบางส่วนได้หรือไม่ ให้พิจารณาจัดทำเอกสารเหตุผลสำหรับพฤติกรรมปัจจุบันหรือตรวจสอบความถูกต้อง allowedSender
ที่อยู่นอกขอบเขตของการกรอกครั้งแรกเพื่อให้แน่ใจว่าจะได้รับการตรวจสอบทุกครั้งที่พยายามเติม
ปรับปรุง: แก้ไขใน ดึงคำขอ #58.
[H03] ผู้ผลิตที่เป็นอันตรายสามารถใช้ประโยชน์จากการเติมบางส่วนเพื่อขโมยทรัพย์สินของผู้รับ
คำสั่งซื้อจาก OrderMixin
สัญญามีความสามารถในการเติมเต็มบางส่วน เพื่อรองรับการเติมบางส่วน โปรโตคอลต้องใช้วิธีการคำนวณการสวอปทั้งสองด้าน ทั้งคู่ getMakerAmount
และ getTakerAmount
ฟิลด์ถูกกำหนดโดยผู้สร้างคำสั่งเพื่อจุดประสงค์ที่แน่นอนนี้
เมื่อกรอกคำสั่งซื้อ ผู้รับจะต้องจัดเตรียม makingAmount
หรือ takingAmount
ค่าเช่นเดียวกับ a thresholdAmount
ค่า. มีโค้ดพาธที่แตกต่างกันสองแบบที่สามารถใช้ได้ ขึ้นอยู่กับว่า makingAmount
หรือ takingAmount
ได้จัดเตรียมไว้ให้
อย่างแรกคือเมื่อ makingAmount
พารามิเตอร์ถูกกำหนด มันสามารถ ตัด makingAmount
คุณค่าและยัง คำนวณ takingAmount
คุ้มค่าสำหรับมัน ในสถานการณ์เช่นนี้ thresholdAmount
ทำให้มั่นใจได้ว่าไฟล์ takingAmount
มูลค่าที่ใช้คือ ไม่ใหญ่เกินคาด.
อันที่สองคือเมื่อ takingAmount
พารามิเตอร์ถูกกำหนด ในกรณีเช่นนี้ จะ คำนวณ makingAmount
มูลค่าด้วยความเป็นไปได้ของ ตัดทอนมัน และ กำลังคำนวณใหม่ takingAmount
ค่าหากสิ่งนั้นเกิดขึ้น ในสถานการณ์เช่นนี้ thresholdAmount
ค่าทำให้มั่นใจได้ว่า makingAmount
ค่าที่ส่งคืนคือ ไม่เล็กอย่างกะทันหัน.
มีวิธีการหาประโยชน์สองวิธี แต่ละวิธีไม่ซ้ำกันกับเส้นทางรหัสที่กล่าวถึงก่อนหน้านี้ วิธีการแสวงหาผลประโยชน์เหล่านี้ต้องการอันตราย getMakerAmount
และ getTakerAmount
ฟังก์ชั่น. การใช้ฟังก์ชันเหล่านี้อย่างง่ายจะมีพฤติกรรมเหมือนกันกับ AmountCalculator
's getMakerAmount
และ getTakerAmount
แต่ด้วยสวิตช์แบบฮาร์ดโค้ดที่จะบังคับให้ส่งคืนค่าที่ควบคุมโดยผู้โจมตีเมื่อจำเป็น
รูปแบบการหาช่องโหว่รูปแบบแรกที่มีความรุนแรงน้อยกว่านั้นเกี่ยวข้องกับเส้นทางรหัสแรกที่ makingAmount
มีการระบุค่า ในลำดับการเติม ผู้ผลิตที่เป็นอันตรายจะรอคำสั่งเติมซึ่งระบุ makingAmount
เพื่อแสดงใน mempool เพื่อ frontrun พวกเขาจะระบายมูลค่าทั้งหมดยกเว้น 1 จากด้านผู้ผลิตแล้วบังคับ _callGetTakerAmount
เพื่อคืนจำนวนเงินที่ระบุในของผู้ใช้ thresholdAmount
มูลค่า (หรือค่าเผื่อหากน้อยกว่า) เมื่อการทำธุรกรรมของผู้ใช้สำเร็จในที่สุด พวกเขาจะสลับเต็มจำนวน thresholdAmount
คุณค่าของ takerAsset
สำหรับหน่วยเดียวของ makerAsset
. การหาประโยชน์นี้ถูกจำกัดโดยจำนวนเงินที่ได้รับจาก thresholdAmount
มูลค่าหรือปริมาณของ takerAsset
ผู้ใช้ได้รับอนุญาตบน LimitOrderProtocol
สัญญา.
รูปแบบการเอารัดเอาเปรียบที่สองที่รุนแรงกว่านั้นเกี่ยวข้องกับเส้นทางรหัสที่สองโดยที่ takingAmount
มีการระบุค่า. ผู้ผลิตที่ประสงค์ร้ายจะรอคำสั่งซื้อที่ระบุ a . ในทำนองเดียวกัน takingAmount
ค่าที่จะแสดงใน mempool พวกเขาจะเป็นผู้นำในการทำธุรกรรมและบังคับให้ makingAmount
ค่าที่ส่งคืนโดย _callGetMakerAmount
ฟังก์ชันให้สูงกว่าทั้ง remainingMakerAmount
และ thresholdAmount
. พวกเขายังจะตั้งค่า takingAmount
คืนค่าโดย _callGetTakerAmount
เป็นจำนวน takerAsset
ทรัพย์สินที่ได้รับอนุญาตบน LimitOrderProtocol
โดยผู้รับ เมื่อการทำธุรกรรมของผู้รับผ่านไป มันจะ ตัดทอน makingAmount
ค่าแล้ว คำนวณใหม่ takingAmount
ค่า. อย่างไรก็ตาม การคำนวณใหม่นี้ไม่รับประกันว่าจะลดลง และในกรณีนี้จะทำให้ผู้รับทั้งหมดหมด takerAsset
ที่พวกเขาอนุญาตในสัญญา ในเส้นทางรหัสนี้ the thresholdAmount
มูลค่าคือ ทำให้มั่นใจว่า makingAmount
ไม่ต่ำเกินไป, เลยเอาของผู้รับทั้งหมด takerAsset
เนื้อหาไม่ถูกตรวจสอบ เงินที่เสียไปนั้นถูกจำกัดด้วยจำนวนเงินของ takerAsset
ทรัพย์สินที่ผู้ใช้อนุญาตบน LimitOrderProtocol
สัญญา.
การหาประโยชน์เหล่านี้ไม่สามารถทำได้หากไม่มีคำสั่งบางส่วน และโดยเฉพาะอย่างยิ่ง คำสั่งบางส่วนที่เป็นอันตราย getMakerAmount
และ getTakerAmount
การใช้งาน
ประเด็นหลักของ thresholdAmount
การตรวจสอบค่าคือครอบคลุมเพียงด้านใดด้านหนึ่งของการแลกเปลี่ยน แต่อีกด้านหนึ่งสามารถจัดการได้ผ่าน frontrunning ไม่มีการรับประกันว่ามูลค่าที่ผู้รับเสนอในตอนแรกยังคงไม่เปลี่ยนแปลง พิจารณาถอด makingAmount
การตัดทอนจากโค้ดพาธทั้งสองและการย้อนกลับหากคำสั่งซื้อไม่รองรับการเติมขนาดใหญ่ตามที่ร้องขอ โดยการทำเช่นนี้ thresholdAmount
สามารถใช้เพื่อจำกัดอีกด้านหนึ่งของการแลกเปลี่ยนอย่างเพียงพอและหลีกเลี่ยงพฤติกรรมที่ไม่คาดคิด แม้จะอยู่ในคำสั่งที่เป็นอันตราย
ปรับปรุง: แก้ไขใน ดึงคำขอ #83.
ความรุนแรงปานกลาง
[M01] อาร์กิวเมนต์คงที่ส่งผ่านหลังจากอาร์กิวเมนต์ไดนามิก
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร OrderMixin
สัญญา getTakerAmount
และ getMakerAmount
ฟิลด์ไบต์ถูกใช้เป็นอาร์กิวเมนต์สำหรับ _callGetTakerAmount
และ _callGetMakerAmount
ฟังก์ชั่น. การเรียกเหล่านี้เป็นวิธีคำนวณด้านหนึ่งของสวอปโดยอิงจากอีกด้านหนึ่ง และอนุญาตให้ผู้ใช้กรอกคำสั่งซื้อได้บางส่วน
พื้นที่ getTakerAmount
/getMakerAmount
ฟิลด์เป็นตัวแปรไดนามิกและบรรจุอยู่ด้านหน้า takerAmount
และ makerAmount
ค่าใน _callGetTakerAmount
และ _callGetMakerAmount
ฟังก์ชั่น. เป็นไปได้สำหรับผู้ผลิตที่เป็นอันตรายเพื่อให้ข้อมูลมากกว่าที่คาดไว้ใน getTakerAmount
และ getMakerAmount
สนามที่จะผลักดัน takerAmount
และ makerAmount
ไบต์ที่ผ่านมาซึ่งถือว่าถูกถอดรหัสในฟังก์ชันถัดไป วิธีนี้ช่วยให้ผู้ผลิตเปลี่ยนจำนวนผู้รับหรือผู้ส่งที่ส่งผ่านไปทางขวาเต็มจำนวนไบต์ และแม้กระทั่งแทนที่ทั้งหมดหากมีข้อมูลเพิ่มเติม 32 ไบต์
ผู้ใช้ต้องตรวจสอบ .ด้วยตนเองแล้ว getTakerAmount
และ getMakerAmount
ฟิลด์ตามลำดับ แต่เทคนิคนี้ค่อนข้างยากที่จะมองเห็น ที่น่าสังเกตก็คือ การโจมตีนี้มีผลกับผู้ที่ไว้ใจได้ภายในด้วย getMakerAmount
และ getTakerAmount
ฟังก์ชั่น. สำหรับการโจมตีส่วนใหญ่ การระบุจำนวนเกณฑ์ที่เหมาะสมจะป้องกันการสูญเสียเงิน
เพื่อป้องกันสิ่งนี้ ให้พิจารณาการเข้ารหัสอาร์กิวเมนต์สแตติกก่อนอาร์กิวเมนต์ไดนามิกเพื่อหลีกเลี่ยงการให้อาร์กิวเมนต์ไดนามิกมีวิธีการควบคุมอาร์กิวเมนต์สแตติก
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
เราจะดูแลเป็นพิเศษด้วยการตรวจสอบความถูกต้องของผู้รับ เราจะพยายามใช้การตรวจสอบความถูกต้องของ getters ใน sdk ของเรา ซึ่งจะช่วยในการกรองคำสั่งซื้อที่อาจเป็นอันตราย
[M02] คำสั่ง ERC721 สามารถจัดการได้
เป็นไปได้ที่จะแลกเปลี่ยนมากกว่าเพียงแค่ ERC20s ผ่านทาง OrderMixin
โดยปรับใช้สัญญาที่ใช้ตัวเลือกฟังก์ชั่นเดียวกับ IERC20's transferFrom
และให้สัญญานั้นเป็น makerAsset
หรือ takerAsset
ในการสั่งซื้อ
ผู้รับมอบฉันทะที่อยู่นอกขอบเขต กล่าวคือ ERC721Proxy
, ERC721ProxySafe
และ ERC1155Proxy
สัญญาเป็นไปตามรูปแบบนี้เพื่อรองรับ ERC721
และ ERC1155
โทเค็น เนื่องจากต้องเรียกผู้รับมอบฉันทะที่มีรูปแบบเดียวกับ IERC20 transferFrom
โทร ลายเซ็นต้องขึ้นต้นด้วย address from
, address to
และ uint256 amount
. สิ่งอื่นใดที่ผู้รับมอบฉันทะต้องการสามารถส่งต่อไปได้และถูกกำหนดในลำดับเป็น makerAssetData
และ takerAssetData
.
ERC1155 สามารถโอนโทเค็น ID เดียวกันได้หลายรายการพร้อมกัน ซึ่งหมายความว่า ERC1155Proxy
สัญญาใช้ประโยชน์จาก amount
สนาม. ในทางกลับกัน, ERC721
ไม่ได้มีการใช้งานที่ชัดเจนสำหรับ amount
สนาม. เนื่องจากเป็นตัวแทนของโทเค็นที่ไม่สามารถเปลี่ยนได้ tokenId เฉพาะจะมีเพียงหนึ่งรายการที่มีอยู่ แสดงผล amount
ฟิลด์ไร้ประโยชน์ ด้วยเหตุนี้การดำเนินการสำหรับทั้งสอง ERC721Proxy
และ ERC721ProxySafe
สัญญาใช้ที่จำเป็น amount
ฟิลด์เป็น tokenId
แทน.
โอเวอร์โหลดนี้ของ amount
พารามิเตอร์สร้างความเป็นไปได้ของการเติมบางส่วน ERC721
คำสั่งซื้อเพื่อซื้อโทเค็นที่ระบุไว้แยกต่างหากในราคาพิเศษ ตัวอย่างเช่น อาจมีกรณีที่ผู้ใช้คนเดียวมีหลายราย ERC721
ของสัญญาเดียวกันที่ได้รับอนุญาตให้โอนโดย ERC721Proxy
สัญญาและแสดงรายการไว้ในคำสั่งจำกัดที่แยกจากกัน
หากคำสั่งจำกัดยังให้ getMakerAmount
และ getTakerAmount
ฟิลด์ จะสามารถกรอกบางส่วนเหล่านี้ได้ ERC721
คำสั่งซื้อ เนื่องจากคำสั่งของ amount
ฟิลด์จริงสอดคล้องกับ tokenId
ผู้ใช้ที่ประสงค์ร้ายสามารถกรอกข้อมูลบางส่วนบน ERC721
ด้วยรหัสโทเค็นที่สูงกว่า ส่งผลให้ a makingAmount
/takingAmount
ของ ERC721
ที่สามารถสอดคล้องกับค่าที่ต่ำกว่า tokenId
. ผลที่ได้คือ ERC721
กับด้านล่าง tokenId
จะโอนในราคา (higher tokenId price) * (lower tokenId's id) / (higher tokenId's id)
.
การใช้ประโยชน์จากช่องโหว่นี้มีข้อกำหนดบางประการ:
- แพลตฟอร์มที่หลากหลาย
ERC721
จากสัญญาเดียวกันจะได้รับอนุญาตบนทั้งERC721
พร็อกซี่โดยเจ้าของคนเดียว - เปิดรับออเดอร์
ERC721
ที่ไม่ต่ำสุดtokenId
ของที่อนุญาต - อนุญาตให้เติมบางส่วนในคำสั่งซื้อ
เพื่อขจัดความเป็นไปได้ของบางส่วนโดยสิ้นเชิง ERC721
เติมพิจารณาแยก amount
และ tokenId
อาร์กิวเมนต์ ไม่ว่าอาร์กิวเมนต์จะถูกแยกออกหรือไม่ก็ตาม ให้พิจารณาจัดทำเอกสารนี้เพื่อเตือนผู้ใช้เกี่ยวกับพฤติกรรมนี้และเพื่อหลีกเลี่ยงรูปแบบนี้ในอนาคต
ปรับปรุง: แก้ไขใน ดึงคำขอ #59.
[M03] สมมติฐานทศนิยมที่ไม่มีเอกสาร
พื้นที่ LimitOrderProtocol
สัญญาสืบทอด ChainlinkCalculator
สัญญาผ่าน OrderMixin
สัญญา. สัญญานี้เปิดเผยสองหน้าที่เพื่อให้สามารถใช้งาน Chainlink oracles ได้ในระหว่าง การตรวจสอบภาคแสดง และการค้นหาของ จำนวนผู้ผลิต/จำนวนผู้รับ.
อย่างไรก็ตาม สัญญาสร้างสมมติฐานที่ไม่มีเอกสารเกี่ยวกับจำนวนทศนิยมที่ Chainlink oracles ควรรายงาน เช่นเดียวกับจำนวนทศนิยมที่พารามิเตอร์ฟังก์ชันควรมี ในบางสถานการณ์ สิ่งนี้อาจนำไปสู่พฤติกรรมที่ไม่คาดคิด รวมถึงการตั้งราคาสินทรัพย์ผิดและการสูญเสียเงินทุนโดยไม่ได้ตั้งใจ
โดยเฉพาะอย่างยิ่ง ตลอดสัญญา ข้อสันนิษฐานโดยปริยายคือ Chainlink oracles จะรายงานด้วยทศนิยม 18 ตำแหน่ง อย่างไรก็ตาม ไม่ ออราเคิล Chainlink ทั้งหมด รายงานด้วยจำนวนทศนิยมนี้ ในความเป็นจริง หาก oracle รายงานคู่โทเค็นที่เป็นสกุลเงิน (เช่น USD) ก็จะมีทศนิยมเพียง 8 ตำแหน่งเท่านั้น เนื่องจากไม่มีข้อจำกัดเกี่ยวกับ ที่ สามารถใช้ oracles ได้ ไม่ควรตั้งสมมติฐานโดยปริยายเกี่ยวกับจำนวนทศนิยมที่จะรายงานด้วย
มีสมมติฐานโดยปริยายว่า amount
พารามิเตอร์สำหรับ ChainlinkCalculator
ฟังก์ชั่นจะใช้ทศนิยม 18 ตำแหน่งพร้อมกับการประกาศที่ชัดเจนที่ทำให้เข้าใจผิดว่า singlePrice
ฟังก์ชัน Calculates price of token relative to ETH scaled by 1e18
. แท้จริงแล้วแม้กับพระอุปัฏฐากนั้น ทำ รายงานด้วยทศนิยม 18 ตำแหน่ง ค่าส่งคืนของ singlePrice
ฟังก์ชันจะถูกปรับขนาดด้วยจำนวนทศนิยมของ amount
พารามิเตอร์ ซึ่งไม่จำเป็นต้องเป็นทศนิยม 18 ตำแหน่ง
ในทำนองเดียวกัน doublePrice
ฟังก์ชันจะถือว่าออราเคิลของ Chainlink สองตัวรายงานด้วยจำนวนทศนิยมเท่ากัน ทำให้ผลลัพธ์ของฟังก์ชันเบี่ยงเบนไปจากความคาดหวัง
พิจารณาจัดทำเอกสารสมมติฐานอย่างชัดเจนเกี่ยวกับจำนวนทศนิยมที่พารามิเตอร์และค่าส่งคืนควรอยู่ในเงื่อนไข นอกจากนี้ ให้พิจารณาจำกัดการคำนวณที่ขึ้นอยู่กับคำทำนายที่ผิดสมมติฐาน หรือให้การคำนวณที่เกี่ยวข้องพิจารณาจำนวนทศนิยมตามจริง
ปรับปรุง: แก้ไขใน ดึงคำขอ #75.
ความรุนแรงต่ำ
[L01] ค่าคงที่ไม่ได้ประกาศอย่างชัดเจน
มีการใช้ค่าตามตัวอักษรเพียงไม่กี่ครั้งโดยมีความหมายที่ไม่สามารถอธิบายได้ใน codebase ตัวอย่างเช่น:
- ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
OrderMixin
สัญญา_remaining
การทำแผนที่มีความหมายมากเกินไป (ตามที่อธิบายไว้ในปัญหา ความหมายของการทำแผนที่มากเกินไป) เพื่อติดตามจำนวนสินทรัพย์คงเหลือสำหรับคำสั่งซื้อที่เติมบางส่วน และ หากคำสั่งซื้อเต็มแล้ว โดยเฉพาะ0
หมายความว่าไม่มีการเติมที่เกี่ยวข้องกับคำสั่งซื้อ1
หมายความว่าไม่สามารถบรรจุคำสั่งซื้อได้อีกต่อไปและสิ่งใดที่ใหญ่กว่า1
หมายความว่ามียอดเงินคงเหลือที่เกี่ยวข้องกับคำสั่งซื้อที่สามารถเติมได้ - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
ChainlinkCalculator
สัญญามูลค่าตามตัวอักษร1e18
ใช้ในไฟล์singlePrice
ฟังก์ชัน
เพื่อปรับปรุงความสามารถในการอ่านโค้ดและอำนวยความสะดวกในการจัดโครงสร้างใหม่ ให้พิจารณากำหนดค่าคงที่สำหรับเลขมหัศจรรย์ทุกตัว ตั้งชื่อให้ชัดเจนและเข้าใจง่าย สำหรับค่าที่ซับซ้อน ให้พิจารณาเพิ่มความคิดเห็นแบบอินไลน์เพื่ออธิบายวิธีคำนวณหรือเหตุผลที่เลือก
ปรับปรุง: แก้ไขใน ดึงคำขอ #75 และ ดึงคำขอ #76.
[L02] ฝ่ายที่เป็นอันตรายอาจป้องกันการดำเนินการตามคำสั่งที่อนุญาตได้
พื้นที่ OrderMixin
สัญญาอนุญาตให้ผู้ใช้ผู้ผลิตส่ง คำสั่งอนุญาต เพื่อให้สามารถดำเนินการได้ในรายการเดียว แทนที่จะต้องมีธุรกรรมแยกต่างหากสำหรับการอนุมัติ นอกจากนี้ คนรับออร์เดอร์สามารถ ยื่นใบอนุญาตของตัวเอง ในระหว่างการกรอกคำสั่งเพื่อวัตถุประสงค์เดียวกัน
อย่างไรก็ตามเนื่องจากใบอนุญาตของผู้ทำมีอยู่ภายใน ใบสั่งทั้งใบอนุญาตของผู้ผลิตและผู้รับจะสามารถเข้าถึงได้ในขณะที่ธุรกรรมการเติมคำสั่งซื้ออยู่ใน mempool สิ่งนี้จะทำให้ผู้ใช้ที่ประสงค์ร้ายสามารถใช้ใบอนุญาตเหล่านั้นและดำเนินการตามสัญญาสินทรัพย์ที่เกี่ยวข้องในขณะที่ดำเนินการธุรกรรมการเติมล่วงหน้า เนื่องจากใบอนุญาตเหล่านี้มี nonce
เพื่อป้องกันการใช้จ่ายซ้ำซ้อน ธุรกรรมการเติมคำสั่งซื้อจะล้มเหลวอันเป็นผลมาจากการพยายามใช้ใบอนุญาตแบบเดิมที่เพิ่งใช้ในระหว่างการดำเนินการ
แม้ว่าจะไม่มีความเสี่ยงด้านความปลอดภัย และผู้ผลิตสามารถสร้างคำสั่งซื้อใหม่และอนุมัติการทำธุรกรรมล่วงหน้า การโจมตีนี้อาจส่งผลกระทบต่อการใช้งานของคำสั่งซื้อที่อนุญาตได้อย่างแน่นอน อันที่จริง ผู้โจมตีที่มีแรงจูงใจสามารถบล็อกได้ ทั้งหมด คำสั่งอนุญาตด้วยการโจมตีครั้งนี้ พิจารณาตรวจสอบว่าได้ส่งใบอนุญาตแล้วหรือหากค่าเผื่อเพียงพอในระหว่างการกรอกคำสั่งซื้อ พิจารณาให้ผู้ใช้ทราบเกี่ยวกับการโจมตีที่เป็นไปได้นี้ในระหว่างการจัดองค์ประกอบคำสั่งซื้อ
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
เรามีการตรวจสอบการอนุมัติมาก่อน แต่ตัดสินใจที่จะลดความซับซ้อนของขั้นตอนการอนุญาตเพื่อเปลี่ยนกลับเมื่อการอนุมัติที่ไม่ประสบความสำเร็จ เราจะคิดถึงวิธีแจ้งปัญหาให้ผู้ผลิตทราบ
[L03] รหัสซ้ำ
มีโค้ดที่ซ้ำกันใน codebase รหัสที่ซ้ำกันอาจนำไปสู่ปัญหาในภายหลังในวงจรการพัฒนา และทำให้โครงการมีแนวโน้มที่จะเกิดข้อผิดพลาดมากขึ้น ข้อผิดพลาดดังกล่าวอาจเกิดขึ้นโดยไม่ได้ตั้งใจเมื่อไม่มีการจำลองการเปลี่ยนแปลงการทำงานในทุกอินสแตนซ์ของโค้ดที่ควรจะเหมือนกัน ตัวอย่างของรหัสที่ซ้ำกัน ได้แก่:
แทนที่จะสร้างรหัสซ้ำ ให้พิจารณาให้มีสัญญาหรือไลบรารีเพียงฉบับเดียวที่มีรหัสที่ซ้ำกัน และใช้งานเมื่อใดก็ตามที่จำเป็นต้องใช้ฟังก์ชันที่ซ้ำกัน
ปรับปรุง: แก้ไขบางส่วนใน ดึงคำขอ #60.
[L04] ชุดทดสอบที่ผิดพลาดหรือทำให้เข้าใจผิด
มีบางกรณีในชุดทดสอบที่การทดสอบเบี่ยงเบนไปจากการทำงานที่คาดไว้ ตัวอย่างเช่น:
- พื้นที่
ChainlinkCalculator
สัญญาเป็นมรดกโดยOrderMixin
สัญญา. อย่างไรก็ตาม ในระหว่างการทดสอบAmountCalculator.arbitraryStaticCall
ใช้เรียกฟังก์ชันChainlinkCalculator
สัญญาเป็นสัญญาภายนอกที่เป็นอิสระ แม้ว่าผลลัพธ์จะเป็นไปตามที่คาดหวัง แต่การทดสอบควรสะท้อนถึงพฤติกรรมกับการออกแบบปัจจุบันของระบบและกรณีการใช้งานที่คาดไว้โดยการเรียกChainlinkCalculator
ทำงานโดยตรงโดยไม่ต้องใช้การเรียกคงที่โดยพลการ - แม้ว่าสัญญาพร็อกซี่จะอยู่นอกขอบเขต แต่เราสังเกตเห็นว่าเมื่อทดสอบโปรโตคอลด้วยทรัพย์สิน ERC721
ERC721Proxy
สัญญาไม่ได้ใช้เพื่อแลกเปลี่ยนสินทรัพย์ใน ชุดทดสอบ.
เนื่องจากชุดทดสอบนั้นอยู่นอกขอบเขตของการตรวจสอบนี้ โปรดพิจารณาตรวจสอบชุดทดสอบอย่างละเอียดเพื่อให้แน่ใจว่าการทดสอบทั้งหมดดำเนินการได้สำเร็จตามข้อกำหนดของโปรโตคอล
ปรับปรุง: แก้ไขใน ดึงคำขอ #57, ดึงคำขอ #59และ ดึงคำขอ #61.
[L05] ข้อผิดพลาดและการละเว้นในเหตุการณ์
ทั่วทั้งฐานรหัส โดยทั่วไปแล้วเหตุการณ์ต่างๆ จะถูกปล่อยออกมาเมื่อมีการเปลี่ยนแปลงที่ละเอียดอ่อนในสัญญา อย่างไรก็ตาม หลายเหตุการณ์ไม่มีพารามิเตอร์ที่จัดทำดัชนีและ/หรือไม่มีพารามิเตอร์ที่สำคัญ ตัวอย่างเช่น:
นอกจากนี้ยังมีการดำเนินการที่มีความละเอียดอ่อนซึ่งไม่มีเหตุการณ์ เช่น:
พิจารณาสร้างดัชนีเหตุการณ์ที่มีอยู่ให้สมบูรณ์ยิ่งขึ้นและเพิ่มพารามิเตอร์ใหม่ที่ขาดไป นอกจากนี้ ให้พิจารณาปล่อยเหตุการณ์ทั้งหมดในลักษณะที่สมบูรณ์ที่สามารถนำมาใช้เพื่อสร้างสถานะของสัญญาใหม่โดยบริการนอกเครือข่าย
ปรับปรุง: ไม่ได้รับการแก้ไข อย่างไรก็ตามทีม 1inch ได้เพิ่ม an orderRemaining
เป็นพารามิเตอร์ OrderCanceled
เหตุการณ์ใน ดึงคำขอ #62.
ทีมงาน 1 นิ้ว กล่าวว่า:
เราพบว่าจำเป็นต้องมีเพียงชุดย่อยของข้อมูลที่จำกัดเพื่อตอบสนองความต้องการส่วนหน้า ในกรณีของการวิเคราะห์อย่างละเอียด ฟิลด์ที่แนะนำทั้งหมดจะพร้อมใช้งานผ่านการติดตาม สำหรับ
OrderRFQMixin
เราคาดหวังให้ผู้ดูแลสภาพคล่องสร้างวิธีการที่ซับซ้อนในการติดตามคำสั่งซื้อที่ถูกยกเลิก
[L06] การเปลี่ยนแปลงการจัดเก็บในระหว่างการปล่อยเหตุการณ์
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร NonceManager
สัญญาเมื่อ NonceIncreased
เหตุการณ์ถูกปล่อยออกมา nonce ของผู้ส่งข้อความก็เพิ่มขึ้นเช่นกัน.
การดำเนินการหลายรายการพร้อมกันอาจทำให้ codebase ยากต่อการให้เหตุผล มีแนวโน้มที่จะเกิดข้อผิดพลาดมากขึ้น และอาจนำไปสู่การมองข้ามหรือเข้าใจผิดในการดำเนินการ
เพื่อปรับปรุงความตั้งใจโดยรวม ความสามารถในการอ่าน และความชัดเจนของโค้ด ให้พิจารณาเพิ่มค่า nonce ก่อนปล่อยเหตุการณ์
ปรับปรุง: แก้ไขใน ดึงคำขอ #63.
[L07] วิธีการถอดรหัสที่ไม่สอดคล้องกันอาจทำให้เกิดความคลาดเคลื่อนของผลลัพธ์
เพื่อรองรับการขยายและความยืดหยุ่นทั้งหมด Limit Order Protocol จึงต้องจัดการกับข้อมูลไบต์แบบไดนามิกและค่าที่ส่งกลับโดยพลการจากสัญญาภายนอก เป็นผลให้โปรโตคอลรวมถึง an ArgumentsDecoder
ไลบรารีเพื่อแปลงค่าไบต์แบบไดนามิกเป็นชนิดข้อมูลพื้นฐานได้อย่างมีประสิทธิภาพมากขึ้น อย่างไรก็ตาม ห้องสมุดนี้ไม่ได้ใช้เฉพาะ และในบางกรณี abi.decode
มาใช้แทน นอกจากนี้ สัญญาบางฉบับกำลังใช้ abi coder v1
ในขณะที่คนอื่นกำลังใช้ abi coder v2
. อดีตดำเนินการคล้ายกับ .มากขึ้น ArgumentsDecoder
ห้องสมุดในขณะที่หลังทำการตรวจสอบเพิ่มเติมเมื่อถอดรหัส
การใช้วิธีการถอดรหัสที่แตกต่างกันเหล่านี้อย่างไม่สอดคล้องกันอาจส่งผลให้เกิดความคลาดเคลื่อนเล็กน้อยระหว่างความตั้งใจและพฤติกรรมที่แท้จริงของโค้ดเบส
ยกตัวอย่างเช่น simulateCalls
ฟังก์ชั่นใช้เฉพาะ ArgumentsDecoder.decodeBool
ฟังก์ชัน ถ้า simulateCalls
ฟังก์ชันถูกใช้เพื่อตรวจสอบการเรียกที่จะทำในส่วนเพรดิเคตของคำสั่ง จากนั้นผลลัพธ์อาจเบี่ยงเบนไปจากสิ่งที่เกิดขึ้นจริงเมื่อเงื่อนไขเพรดิเคตถูกประเมิน เนื่องจากมีการใช้วิธีการถอดรหัสที่แตกต่างกัน
ตัวอย่างเช่น ถ้าภาคแสดงทำให้เป็นexternal staticcall
ไปยังฟังก์ชันบางอย่างที่คืนค่า a uint256
มีค่ามากกว่าหนึ่งมากกว่าที่คาดไว้ bool
จากนั้นการโทรนั้นจะเปลี่ยนกลับเนื่องจากค่าที่ส่งคืนคือ ถอดรหัสด้วย abi coder v2
's abi.decode
ซึ่งจะไม่ยอมรับค่าเช่น bool
. อย่างไรก็ตาม หากใช้สายเดียวกันกับ simulateCalls
แล้วมัน จะถูกทำเครื่องหมายเป็น true
, เพราะ decodeBool
ถือว่าค่าใดๆ ที่มากกว่าศูนย์เป็น true
.
ที่จะทำให้ simulateCalls
ทำหน้าที่สะท้อนพฤติกรรมของการเรียกเพรดิเคตจริงอย่างสมบูรณ์ พิจารณาแก้ไขเพื่อใช้ abi.decode
.
ปรับปรุง: แก้ไขใน ดึงคำขอ #82.
[L08] ขาดการตรวจสอบอินพุต
พื้นที่ fillOrderToWithPermit
และ fillOrderTo
ฟังก์ชั่นของ OrderMixin
สัญญา เช่นเดียวกับ fillOrderRFQToWithPermit
และ fillOrderRFQTo
ฟังก์ชั่นของ OrderRFQMixin
สัญญาไม่ตรวจสอบ target
พารามิเตอร์ที่อยู่
ซึ่งทำให้ผู้ใช้สามารถส่งผ่านที่อยู่ศูนย์โดยไม่ตั้งใจ และทำให้ล็อกทรัพย์สินที่พวกเขาควรจะได้รับหลังจากกรอกคำสั่งซื้อโดยไม่ได้ตั้งใจ
เพื่อให้แน่ใจว่าผู้ใช้ไม่ได้ล็อกเงินโดยไม่ได้ตั้งใจ ให้พิจารณาตรวจสอบว่า target
ที่อยู่ไม่เท่ากับที่อยู่ในศูนย์ในฟังก์ชันที่อ้างถึง
ปรับปรุง: แก้ไขใน ดึงคำขอ #78.
[L09] ความครอบคลุมการทดสอบหน่วยต่ำ
ความครอบคลุมการทดสอบหน่วยสำหรับโครงการทั้งหมดอยู่ที่ประมาณ 75% โดยสัญญาบางฉบับมีความครอบคลุมต่ำเป็นพิเศษ
เมื่อพิจารณาถึงความสำคัญของการทดสอบหน่วยเพื่อตรวจสอบรหัสและป้องกันการถดถอยเมื่อจัดโครงสร้างใหม่และพัฒนาคุณลักษณะใหม่ เราสนับสนุนให้เพิ่มความครอบคลุมการทดสอบหน่วยเป็นอย่างน้อย 95% อย่างมีนัยสำคัญ และรวมถึงกรณีของขอบที่ครอบคลุมแม้สถานการณ์ที่ไม่น่าจะเกิดขึ้น
ปรับปรุง: ไม่ได้รับการแก้ไข
[L10] เอกสารอินไลน์ที่ทำให้เข้าใจผิดหรือไม่สมบูรณ์
ตลอดทั้ง Codebase มีการระบุตัวอย่างเอกสารอินไลน์ที่ทำให้เข้าใจผิดและ/หรือไม่สมบูรณ์สองสามตัวอย่าง และควรได้รับการแก้ไข
ต่อไปนี้เป็นตัวอย่างของเอกสารอินไลน์ที่ทำให้เข้าใจผิด:
- ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
ChainlinkCalculator
สัญญาsinglePrice
ฟังก์ชั่น แนทสเปค@notice
แท็ก บอกว่ามันCalculates price of token relative to ETH scaled by 1e18
แต่แท้จริงแล้วผลลัพธ์คือ ความคุ้มค่า ofamount
โทเค็นที่ปรับขนาดโดย1e18
โดยที่ oracle อาจไม่รายงานในแง่ของ ETH (สำหรับคู่ที่ไม่รวม ETH เป็นต้น) - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
OrderRFQMixin
สัญญาinvalidatorForOrderRFQ
ฟังก์ชั่น แนทสเปค@return
แท็ก ทำให้เข้าใจผิดเพราะอาจไม่ได้เติมใบเสนอราคาสำหรับบิตที่ไม่ถูกต้องตามลำดับที่ได้รับการตั้งค่า คำสั่งซื้อยังสามารถยกเลิกได้ - ออนไลน์ 147, 165และ 188 of
OrderMixin.sol
, NatSpec@param
แท็กผิดไวยากรณ์ - ออนไลน์ 20 of
ERC1155Proxy.sol
ที่@notice
แท็กระบุว่าแฮชที่คำนวณเป็นผลมาจากการแฮชfunc_733NCGU
ฟังก์ชัน ซึ่งควรเป็นfunc_301JL5R
ทำหน้าที่แทน
ต่อไปนี้เป็นตัวอย่างของเอกสารอินไลน์ที่ไม่สมบูรณ์:
- ฟังก์ชั่นใน in
AmountCalculator
สัญญาไม่ได้อธิบายพารามิเตอร์ใด ๆ - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
ChainlinkCalculator
สัญญาsinglePrice
และdoublePrice
ฟังก์ชันไม่ได้อธิบายพารามิเตอร์ทั้งหมด - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
ImmutableOwner
สัญญา ตัวแปรสาธารณะและตัวแก้ไขไม่มี NatSpec - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
InteractiveNotificationReceiver
สัญญาnotifyFillOrder
ฟังก์ชั่นไม่ได้อธิบายพารามิเตอร์ใด ๆ - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
LimitOrderProtocol
สัญญาDOMAIN_SEPARATOR
ฟังก์ชั่นไม่มี NatSpec - เหตุการณ์และการทำแผนที่ใน
NonceManager
ไม่มี NatSpec - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
OrderRFQMixin
สัญญา,cancelOrderRFQ*
ฟังก์ชันไม่ได้อธิบายค่าที่ส่งกลับ - ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร
OrderMixin
สัญญาฟังก์ชั่นหลายอย่างขาด NatSpec ที่สมบูรณ์ - ออนไลน์ 168 of
OrderMixin.sol
และทางไลน์ 71 ofOrderRFQMixin.sol
มันหายไป@dev
แท็ก - ฟังก์ชั่นใน in
PredicateHelper
สัญญาไม่ได้อธิบายพารามิเตอร์ทั้งหมด
เอกสารอินไลน์ที่ชัดเจนเป็นพื้นฐานสำหรับการสรุปจุดประสงค์ของโค้ด ความไม่ตรงกันระหว่างเอกสารอินไลน์และการใช้งานอาจนำไปสู่ความเข้าใจผิดอย่างร้ายแรงเกี่ยวกับวิธีที่ระบบคาดว่าจะทำงาน พิจารณาแก้ไขข้อผิดพลาดเหล่านี้เพื่อหลีกเลี่ยงความสับสนสำหรับนักพัฒนา ผู้ใช้ และผู้ตรวจสอบ
ปรับปรุง: แก้ไขบางส่วน เอกสารที่ทำให้เข้าใจผิดใน ดึงคำขอ #75 และ ดึงคำขอ #77.
ทีมงาน 1 นิ้ว กล่าวว่า:
เราได้แก้ไขเอกสารที่ทำให้เข้าใจผิด เอกสารจะเสร็จสิ้นในภายหลัง
[L11] คำสั่ง DoS เป็นไปได้เมื่อใช้ hooks
พื้นที่ OrderMixin
สัญญาใช้ฟังก์ชันการทำงานเพื่อเติมคำสั่งแลกเปลี่ยนนอกสายโซ่ทั่วไปซึ่งอาจมีเงื่อนไขสำหรับความสำเร็จของพวกเขา ในระหว่างการกรอกคำสั่งซื้อ คำสั่งซื้อสามารถ ตรวจสอบเงื่อนไข "ภาคแสดง" ที่กำหนดไว้ล่วงหน้า ก่อนดำเนินการต่อไป
อย่างไรก็ตาม เนื่องจากเงื่อนไขของเพรดิเคตเหล่านี้สามารถกำหนดเป้าหมายตรรกะของสัญญาใดๆ ก็ได้ ผู้ผลิตที่ประสงค์ร้ายสามารถหลอกให้ผู้รับเชื่อว่าคำสั่งทำงานอย่างถูกต้องและใช้ได้เมื่อตรวจสอบนอกสายโซ่ แต่ล้มเหลวเมื่อพยายามกรอกคำสั่งเดียวกัน บนโซ่ การเปลี่ยนแปลงพฤติกรรมของเพรดิเคตนี้สามารถทำได้โดย frontrunning บางสถานะตัวแปรที่เพรดิเคตขึ้นอยู่กับ โดยการตรวจสอบก๊าซที่ส่งหรือแม้แต่ที่อยู่ที่เกี่ยวข้องกับการโทรหรือโดยตรรกะอื่น ๆ
นอกจากนี้ หากผู้ทำกำหนด a ปฏิสัมพันธ์ระหว่างการแลกเปลี่ยนที่ interactionTarget
สัญญาสามารถย้อนกลับหรือเพิกถอนค่าเผื่อเพื่อป้องกันการกรอกคำสั่งที่ประสบความสำเร็จ โดยหลักแล้วจะนำไปสู่ผลลัพธ์เดียวกันกับภาคแสดงที่เป็นอันตราย
แม้ว่าทรัพย์สินจะไม่ตกอยู่ในความเสี่ยง ผู้ใช้หรือบอทที่ค้นหาคำสั่งซื้อที่น่าพอใจจะมีภาระเพิ่มขึ้นในการพยายามระบุคำสั่งซื้อสแปมที่อาจดูเหมือนถูกต้องตามกฎหมาย ในกรณีที่ไม่สามารถระบุคำสั่งซื้อประเภทนี้ได้ จะส่งผลให้ต้องเสียค่าน้ำมัน หากต้องการลดปริมาณการสั่งซื้อสแปม ให้พิจารณาจำกัดเป้าหมายที่มีอยู่สำหรับ hooks เหล่านี้ พิจารณาเตือนผู้ใช้เกี่ยวกับความเป็นไปได้นี้ก่อนที่จะพยายามเติมคำสั่งซื้อ
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
เราจัดการเรื่องนั้นในแบ็กเอนด์ของเรา และเราจะคิดหาวิธีที่จะแจ้งให้ผู้รับที่เป็นไปได้ทราบเกี่ยวกับปัญหานี้
[L12] การปัดเศษอาจทำให้เสียเปรียบได้ taker
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร OrderMixin
และ OrderRFQMixin
สัญญาเมื่อมีการกรอกคำสั่งและผู้รับให้เฉพาะ a makingAmount
or takingAmount
จำนวนเงิน โปรโตคอลพยายามคำนวณจำนวนคู่ของการแลกเปลี่ยน
มีสองประเด็นในการคำนวณเหล่านี้ ประเด็นแรกคือไม่มีเอกสารหรือตรรกะที่จำกัดจำนวนทศนิยมที่พารามิเตอร์จำนวนควรใช้ ซึ่งเราได้กล่าวถึงใน สมมติฐานทศนิยมที่ไม่มีเอกสาร ปัญหา.
ประเด็นที่สองคือ ในระหว่างการคำนวณเหล่านี้ โปรโตคอลจะเข้ามาแทนที่ผู้สร้าง ปัญหาการปัดเศษสามารถรุนแรงขึ้นได้อย่างมากเมื่อสมมติฐานทศนิยมโดยนัยถูกทำลาย แต่แม้ว่าทุกอย่างจะเป็นไปตามเงื่อนไขที่คาดไว้ การปัดเศษจะเกิดขึ้นด้วยจำนวนเล็กน้อยที่แปลก
พิจารณาให้ผู้รับกำหนดจำนวนเงินขั้นต่ำของ makerAsset
ทรัพย์สินที่ตนยินดีรับร่วมกันเป็นจำนวนเงินสูงสุด takerAsset
สินทรัพย์ที่พวกเขายินดีแลกเปลี่ยน เพื่อให้การยอมรับการปัดเศษมีความชัดเจนมากขึ้น
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
จำนวนเกณฑ์ควรจะเพียงพอสำหรับการคุ้มครองของผู้รับ
[L13] การจัดการคำสั่งซื้อที่ขัดแย้งกันเมื่อไม่มีพารามิเตอร์
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร OrderMixin
สัญญา fillOrderTo
ฟังก์ชั่นทำการเรียกภายในไปยัง _callGetMakerAmount
และ _callGetTakerAmount
ทำงานทุกครั้งที่มีการพยายามเติมและ either makingAmount
หรือ takingAmount
พารามิเตอร์เป็นศูนย์ตามลำดับหรือถ้า makingAmount
มีค่ามากกว่า remainingMakerAmount
มูลค่า
พื้นที่ _callGetMakerAmount
และ _callGetTakerAmount
การโทรจะนำไปสู่การพลิกกลับหากคำสั่งไม่ได้สร้างด้วย getMakerAmount
or getTakerAmount
พารามิเตอร์ตามลำดับและการเติมบางส่วนกำลังดำเนินการ
An แสดงความคิดเห็นแบบอินไลน์ข้าง _callGetMakerAmount
และ แสดงความคิดเห็นแบบอินไลน์ข้าง _callGetTakerAmount
อ้างว่า "อนุญาตให้เติมได้ทั้งหมดเท่านั้น" หากไม่ได้สร้างคำสั่งซื้อด้วย getMakerAmount
or getTakerAmount
พารามิเตอร์
อย่างไรก็ตาม มีเส้นทางของรหัสที่สิ่งนี้ใช้ไม่ได้ เนื่องจากเส้นทางเหล่านั้นไม่ตรวจสอบ length
ของทั้งสอง getMakerAmount
และ getTakerAmount
พารามิเตอร์
โดยเฉพาะ เมื่อ taker
ระบุ a takerAmount
มูลค่าสำหรับคำสั่งซื้อที่มีเพียง a getMakerAmount
เว้นแต่จะเรียกมาที่ getMakerAmount
ส่งกลับจำนวนเงินที่มากกว่า remainingMakerAmount
การเติมบางส่วนสามารถทำได้โดยขัดแย้งกับเอกสารอินไลน์
ซึ่งจะทำให้เจตนาของเส้นทางรหัสเหล่านั้นไม่ชัดเจน หากเป็นลักษณะการทำงานที่คาดไว้ ให้พิจารณาแก้ไขเอกสารอินไลน์เพื่อให้มีความชัดเจนมากขึ้น หากเป็นพฤติกรรมที่ไม่ตั้งใจ ให้พิจารณาตรวจสอบความยาวของทั้งสองเสมอ getMakerAmount
และ getTakerAmount
พารามิเตอร์พร้อมกันเพื่อให้การใช้งานช่วยเสริมพฤติกรรมที่อธิบายไว้ในเอกสารอินไลน์
ปรับปรุง: แก้ไขใน ดึงคำขอ #79.
[L14] การใช้การเรียก Chainlink ที่เลิกใช้แล้ว
พื้นที่ ChainlinkCalculator
สัญญามีวัตถุประสงค์เพื่อใช้ในการค้นหา Chainlink oracles ทำได้โดยการโทรไปที่ .ของพวกเขา latestTimestamp
และ latestAnswer
วิธีการ ซึ่งทั้งสองได้เลิกใช้แล้ว. อันที่จริง เมธอดไม่มีอยู่ใน API ของ Chainlink aggregators อีกต่อไป ในรุ่นสาม.
เพื่อหลีกเลี่ยงความไม่เข้ากันที่อาจเกิดขึ้นกับ Chainlink oracles ในอนาคต ให้พิจารณาใช้ latestRoundData
วิธีการแทน
ปรับปรุง: แก้ไขใน ดึงคำขอ #67.
หมายเหตุและข้อมูลเพิ่มเติม
[N01] ไม่นำเข้าอินเทอร์เฟซ
พื้นที่ AggregatorInterface
อินเทอร์เฟซดูเหมือนจะเป็นชุดย่อยของรหัสที่คัดลอกมาจาก ChainLink
ที่เก็บรหัสสาธารณะของ. อินเทอร์เฟซแบบเต็มรวมอยู่ใน ChainLink
แพ็คเกจ npm ของสัญญา.
เมื่อเป็นไปได้ เพื่อลดโอกาสที่อินเทอร์เฟซไม่ตรงกันและปัญหาผลลัพธ์ แทนที่จะกำหนดใหม่และ/หรือเขียนอินเทอร์เฟซของโครงการอื่นใหม่ ให้พิจารณาใช้อินเทอร์เฟซที่ติดตั้งผ่านแพ็คเกจ npm อย่างเป็นทางการแทน
ปรับปรุง: แก้ไขใน ดึงคำขอ #66.
[N02] เลิกใช้การพึ่งพาโปรเจ็กต์
ระหว่างการติดตั้ง การพึ่งพาของโครงการ, NPM เตือนว่าหนึ่งในแพ็คเกจที่ติดตั้ง Highlight
, “จะไม่ได้รับการสนับสนุนหรือรับการอัปเดตความปลอดภัยอีกต่อไปในอนาคต”
แม้ว่าไม่น่าจะเป็นไปได้ที่แพ็คเกจนี้อาจทำให้เกิดความเสี่ยงด้านความปลอดภัย ให้พิจารณาอัปเกรดการขึ้นต่อกันที่ใช้แพ็คเกจนี้เป็นเวอร์ชันที่บำรุงรักษา
ปรับปรุง: แก้ไขใน ดึงคำขอ #64.
[N03] การเรียกภายนอกเพื่อดูเมธอดไม่ใช่ staticcalls
ทั่วทั้ง codebase ส่วนใหญ่ โปรโตคอลจะทำการเรียกภายนอกอย่างชัดเจนผ่าน OpenZeppelin's functionStaticCall
วิธีจำกัดความเป็นไปได้สำหรับการเปลี่ยนแปลงสถานะเมื่อไม่คาดหวังหรือไม่ต้องการ อย่างไรก็ตาม ใน ChainlinkCalculator
สัญญา ทั้งๆ ที่ตั้งใจจะเรียกภายนอกเท่านั้นถึง view
วิธีการใน Chainlink oracles การเรียกภายนอกใน singlePrice
และ doublePrice
ฟังก์ชั่นไม่ได้ทำผ่านอย่างชัดเจน staticcall
s.
แม้ว่าเราไม่ได้ระบุข้อกังวลด้านความปลอดภัยในทันทีที่เกิดจากสิ่งนี้ เพื่อลดพื้นผิวการโจมตี ปรับปรุงความสอดคล้อง และชี้แจงเจตนา ให้พิจารณาใช้อย่างชัดแจ้ง staticcall
s สำหรับการโทรภายนอกทั้งหมดไปยัง view
ทำหน้าที่ใน ChainlinkCalculator
สัญญา.
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
เราคิดว่าความซับซ้อนของไวยากรณ์ทำให้การปรับปรุงในความสอดคล้องเป็นโมฆะ
[N04] ไม่ล้มเหลวก่อนกำหนดด้วยคำสั่งซื้อที่ไม่ถูกต้อง
ตัว Vortex Indicator ได้ถูกนำเสนอลงในนิตยสาร OrderMixin
สัญญา fillOrderTo
ฟังก์ชั่นจัดการเงื่อนไขพิเศษเมื่อยังไม่ได้ส่งคำสั่งซื้อ (remainingMakerAmount == 0
) แต่ไม่ได้จัดการเงื่อนไขอย่างชัดเจนเมื่อคำสั่งใช้งานไม่ได้อีกต่อไป (remainingMakerAmount == 1
).
ในสถานการณ์หลังนี้ ฟังก์ชันจะเปลี่ยนกลับในที่สุด แต่หลังจากการเผาไหม้ก๊าซในปริมาณเล็กน้อยเท่านั้น เพื่อชี้แจงเจตนา เพิ่มความสามารถในการอ่าน และลดการใช้ก๊าซ ให้พิจารณาการจัดการสถานการณ์ลำดับที่ไม่ถูกต้องอย่างชัดเจนในช่วงเริ่มต้นของฟังก์ชัน
ปรับปรุง: แก้ไขใน ดึงคำขอ #68.
[N05] สัญญาผู้ช่วยไม่ได้ทำเครื่องหมายเป็นนามธรรม
ใน Solidity คีย์เวิร์ด abstract
ใช้สำหรับสัญญาที่ไม่ใช่สัญญาที่ใช้งานได้ในสิทธิของตนเองหรือไม่ได้มีวัตถุประสงค์เพื่อใช้ในลักษณะดังกล่าว แทนที่, abstract
สัญญาได้รับการสืบทอดโดยสัญญาอื่นในระบบเพื่อสร้างสัญญาการทำงานแบบสแตนด์อโลน
ใน Codebase มีตัวอย่างของ Helper contract ที่ไม่ได้กำหนดเป็นนามธรรม แม้ว่าจะไม่ได้ตั้งใจให้ใช้งานด้วยตัวเองก็ตาม ตัวอย่างเช่น AmountCalculator
, ChainlinkCalculator
, ImmutableOwner
, NonceManager
และ PredicateHelper
สัญญาทั้งหมดประกอบด้วยชุดของฟังก์ชันพื้นฐานซึ่งมีวัตถุประสงค์เพื่อใช้โดยการสืบทอดสัญญา
พิจารณาทำเครื่องหมายสัญญาผู้ช่วยเป็น abstract
เพื่อแสดงให้เห็นชัดเจนว่าได้รับการออกแบบมาเพื่อเพิ่มฟังก์ชันการทำงานให้กับสัญญาที่สืบทอดมาเท่านั้น
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
ผู้ช่วยเหล่านั้นสามารถนำไปใช้งานแยกกันได้ พวกมันสืบทอดมาเพื่อการประหยัดก๊าซเท่านั้น
[N06] การจัดลำดับฟังก์ชันไม่สอดคล้องกัน
ตลอด codebase การเรียงลำดับฟังก์ชันโดยทั่วไปจะตามหลัง ลำดับที่แนะนำใน Solidity Style Guide, ซึ่งเป็น: constructor
, fallback
, external
, public
, internal
, private
.
อย่างไรก็ตามในการ OrderMixin
สัญญา public
checkPredicate
ฟังก์ชันเบี่ยงเบนจากคำแนะนำรูปแบบโดยแบ่ง external
ฟังก์ชั่น
เพื่อปรับปรุงความชัดเจนโดยรวมของโปรเจ็กต์ ให้พิจารณาการจัดลำดับฟังก์ชันที่เป็นมาตรฐานทั่วทั้ง codebase ตามคำแนะนำของ Solidity Style Guide
ปรับปรุง: แก้ไขใน ดึงคำขอ #69.
[N07] ขั้นตอนการสั่งซื้อไม่สอดคล้องกัน
พื้นที่ OrderMixin
และ RFQOrderMixin
สัญญาทั้งสองจัดการการกรอกคำสั่งที่ลงนามแล้ว แต่กระแสคำสั่งทั่วไประหว่างสองสัญญานั้นไม่สอดคล้องกัน
OrderMixin
's fillOrderTo
ฟังก์ชั่นตามกระแสทั่วไปนี้ (รหัสหลอก):
if ((takingAmount == 0) == (makingAmount == 0))
else if (takingAmount == 0)
else (handle makingAmount == 0) THEN swapTokens
ในขณะที่ RFQOrderMixin
คล้ายคลึง fillOrderRFQTo
ฟังก์ชั่นตามโฟลว์นี้ (รหัสหลอก):
if (takingAmount == 0 && makingAmount == 0)
else if (takingAmount == 0)
else if (makingAmount == 0)
else revert THEN swapTokens
ไม่มีข้อมูลเชิงลึกจากเอกสารว่าเหตุใดเงื่อนไขแรกในแต่ละฟังก์ชันทั้งสองจึงแตกต่างกัน หรือเพราะเหตุใด takingAmount
และ makingAmount
ไม่สามารถเป็นศูนย์ทั้งคู่ในฟังก์ชันหลังได้ นอกจากนี้ กรณีที่ทั้ง a makingAmount
และ takingAmount
มีให้ง่ายกว่ามากในการให้เหตุผลใน fillOrderRFQTo
เนื่องจากมีการจัดการอย่างชัดเจนในขั้นสุดท้าย else
กลุ่ม
เพื่อชี้แจงเจตนาและเพิ่มความสามารถในการอ่านโค้ดโดยรวม ให้พิจารณาสร้างมาตรฐานลำดับขั้นตอนทั่วไปในสัญญาทั้งสองฉบับ หรือจัดทำเอกสารอย่างชัดเจนว่าเหตุใดจึงมีความแตกต่างกัน
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
นี่เป็นเพราะฟังก์ชันการกำหนดราคาแบบกำหนดเองในคำสั่งซื้อที่จำกัด เนื่องจาก
getMakerAmount
อาจแตกต่างอย่างมากจากgetTakerAmount
เราคิดว่าเป็นการดีกว่าที่จะไม่สร้างตัวเลือกเริ่มต้นให้กับผู้รับ เพราะอาจทำให้พวกเขาสับสนในกรณีที่ผู้รับเหล่านั้นจะแตกต่างกัน
[N08] ข้อความแสดงข้อผิดพลาดมีรูปแบบที่ไม่สอดคล้องกันหรือทำให้เข้าใจผิด
ตลอด codebase นั้น the require
และ revert
ข้อความแสดงข้อผิดพลาดซึ่งมีไว้เพื่อแจ้งให้ผู้ใช้ทราบถึงเงื่อนไขเฉพาะที่ทำให้ธุรกรรมล้มเหลว พบว่ามีรูปแบบที่ไม่สอดคล้องกัน
ตัวอย่างเช่น แต่ละข้อความแสดงข้อผิดพลาดบนบรรทัด 85 จาก OrderMixin.sol
, 16 จาก ERC721ProxySafe.sol
และ 26 จาก Permitable.sol
ใช้สไตล์ที่แตกต่าง
นอกจากนี้ ข้อความแสดงข้อผิดพลาดบางข้อความทำให้เข้าใจผิด:
ข้อความแสดงข้อผิดพลาดมีขึ้นเพื่อแจ้งให้ผู้ใช้ทราบเกี่ยวกับเงื่อนไขความล้มเหลว ดังนั้นพวกเขาจึงควรให้ข้อมูลที่เพียงพอเพื่อให้สามารถแก้ไขอย่างเหมาะสมเพื่อโต้ตอบกับระบบ ข้อความแสดงข้อผิดพลาดที่ไม่ทราบข้อมูลสร้างความเสียหายอย่างมากต่อประสบการณ์ของผู้ใช้โดยรวม ส่งผลให้คุณภาพของระบบลดลง นอกจากนี้ ข้อความแสดงข้อผิดพลาดที่จัดรูปแบบไม่สอดคล้องกันอาจทำให้เกิดความสับสนโดยไม่จำเป็น ดังนั้น ให้พิจารณาตรวจสอบ codebase ทั้งหมดเพื่อให้แน่ใจว่าทุก require
และ revert
คำสั่งมีข้อความแสดงข้อผิดพลาดที่มีรูปแบบสม่ำเสมอ ถูกต้อง ให้ข้อมูลและใช้งานง่าย
ปรับปรุง: แก้ไขบางส่วนใน ดึงคำขอ #81.
[N09] การใช้ตัวแปรส่งคืนที่มีชื่อไม่สอดคล้องกัน
มีการใช้ตัวแปร return ที่มีชื่อไม่สอดคล้องกันใน OrderMixin
สัญญา. ฟังก์ชั่นบางอย่าง ส่งคืนตัวแปรที่มีชื่ออื่น ๆ คืนค่าที่ชัดเจน, และคนอื่น ๆ ประกาศตัวแปร return ที่มีชื่อ แต่แทนที่มัน ด้วยคำสั่งส่งคืนที่ชัดเจน
พิจารณาใช้แนวทางที่สอดคล้องกันในการส่งคืนค่าทั่วทั้ง codebase โดยการลบตัวแปร return ที่มีชื่อทั้งหมด การประกาศให้เป็นตัวแปรในเครื่องอย่างชัดแจ้ง และเพิ่มคำสั่ง return ที่จำเป็นตามความเหมาะสม สิ่งนี้จะปรับปรุงทั้งความชัดเจนและความสามารถในการอ่านของโค้ด และอาจช่วยลดการถดถอยในระหว่างการสร้างโค้ดใหม่ในอนาคต
ปรับปรุง: แก้ไขใน ดึงคำขอ #73.
[N10] การคำนวณแฮชของคำสั่งซื้อไม่เปิดให้ API
พื้นที่ external
ฟังก์ชั่น remaining
, remainingRaw
และ remainingsRaw
ทุกคนคาดหวังคำสั่งแฮชสำหรับการดำเนินการที่ประสบความสำเร็จ
อย่างไรก็ตาม ฟังก์ชันตัวช่วย _hash
ซึ่งส่งคืนแฮชของคำสั่ง has private
ทัศนวิสัย. ซึ่งหมายความว่าผู้ใช้จะต้องแพ็คบางส่วนของคำสั่งซื้อและสตริงโดเมนด้วยตนเองเพื่อรับแฮชของคำสั่งซื้อ
เพื่อหลีกเลี่ยงข้อผิดพลาดที่อาจเกิดขึ้นในการคำนวณแฮชของคำสั่งซื้อ และเพื่อให้ผู้ใช้มีวิธีในการสร้างแฮชตามลำดับของคำสั่งซื้อ ให้พิจารณาขยายการมองเห็นของ _hash
ฟังก์ชั่นเพื่อ public
และเปลี่ยนชื่อใหม่เป็น hash
เพื่อให้สอดคล้องกับรหัสที่เหลือ
ปรับปรุง: แก้ไขใน ดึงคำขอ #74.
[N11] ความหมายของการทำแผนที่มากเกินไป
พื้นที่ _remaining
การทำแผนที่ใน OrderMixin
สัญญามีการใช้งานมากเกินไปในการติดตามสถานะของคำสั่งซื้อและจำนวนสินทรัพย์ที่เหลืออยู่สำหรับคำสั่งซื้อเหล่านั้น
สามสถานะที่สามารถทำได้คือ:
0
: ยังไม่เห็นคำสั่งซื้อที่แฮช1
: คำสั่งซื้อถูกยกเลิกหรือเต็มแล้ว- ใด
uint
ใหญ่กว่า1
: ที่เหลือmakerAmount
มีให้กรอกในการสั่งซื้อ plus1
.
ความหมายเกินพิกัดนี้จำเป็นต้องมีการห่อและแกะค่านี้ในระหว่าง lookup
, cancellation
, initialization
และ storage
.
การโอเวอร์โหลดทางความหมายและตรรกะที่จำเป็นในการเปิดใช้งานอาจมีแนวโน้มที่จะเกิดข้อผิดพลาด และทำให้โค้ดเบสเข้าใจและให้เหตุผลได้ยากขึ้น นอกจากนี้ยังอาจเปิดประตูสำหรับการถดถอยในการอัปเดตโค้ดในอนาคตอีกด้วย
เพื่อปรับปรุงความสามารถในการอ่านโค้ด ให้พิจารณาติดตามสถานะความสมบูรณ์ของคำสั่งซื้อในการแมปแยกต่างหาก
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมขนาด 1 นิ้วอ้างว่าการแก้ไขจะเพิ่มค่าน้ำมันสำหรับ fillOrder
ฟังก์ชัน
[N12] คำสั่งอนุญาตอนุญาตให้เรียกสัญญาตามอำเภอใจ
พื้นที่ OrderMixin
สัญญาสืบทอด Permitable
สัญญาอนุญาตให้มีคำสั่งรายการเดียวกรอกสินทรัพย์ที่ยอมรับดังกล่าว permit
เรียกร้องให้แก้ไขเบี้ยเลี้ยง
อย่างไรก็ตาม โทรไปที่ Permitable
สัญญา ไม่ตรวจสอบว่าเป้าหมายเป็นสินทรัพย์ที่อนุญาตหรือเป็นสินทรัพย์ ซึ่งอาจอนุญาตให้ผู้ใช้ที่ประสงค์ร้ายส่งที่อยู่ของสัญญาโดยอำเภอใจซึ่งสามารถดำเนินการโทรอีกครั้งก่อนที่คำสั่งซื้อจะเสร็จสมบูรณ์
แม้ว่าสัญญาจะเป็น ป้องกันการกลับเข้ามาใหม่แนะนำให้ลดพื้นผิวการโจมตีและป้องกันการเรียกสัญญาภายนอกระหว่างการดำเนินการเสมอ พิจารณาจำกัดสินทรัพย์ที่เกี่ยวข้องกับการอนุญาตสำหรับสินทรัพย์ที่เกี่ยวข้องในคำสั่งหรือรายการสินทรัพย์ที่อนุญาตสำหรับโปรโตคอล
ปรับปรุง: ไม่ได้รับการแก้ไข ทีมงาน 1 นิ้ว กล่าวว่า:
OrderMixin
ไม่มีข้อมูลเกี่ยวกับโทเค็นจริงเช่นmakerAsset
และtakerAsset
บางครั้งเป็นผู้รับมอบฉันทะหรือสัญญาขั้นกลางอื่น ๆ และข้อมูลเกี่ยวกับโทเค็นจริงจะถูกเก็บไว้ในไบต์โดยพลการ จึงไม่มีวิธีใดที่จะจำกัดเนื้อหาใดได้permit
ถูกเรียก
[N13] solhint
ไม่เคยเปิดใช้งานอีกครั้ง
ตลอด codebase มีสองสามตัว solhint-disable
ข้อความโดยเฉพาะในบรรทัด 23 และทางไลน์ 41 of RevertReasonParser.sol
ที่ไม่ลงท้ายด้วย solhint-enable
.
เพื่อประโยชน์ของความชัดเจนและเป็นการจำกัดให้มากที่สุดเมื่อปิดการใช้งาน solhint
ให้พิจารณาใช้ solhint-disable-line
or solhint-disable-next-line
แทน คล้ายกับline 16 ของไฟล์เดียวกัน
ปรับปรุง: แก้ไขใน ดึงคำขอ #72.
[N14] พิมพ์ผิด
Codebase มีการพิมพ์ผิดต่อไปนี้:
นอกจากนี้โครงการของ README
(อยู่นอกขอบเขตสำหรับการตรวจสอบนี้) มีการพิมพ์ผิดต่อไปนี้:
ลองแก้ไขข้อผิดพลาดเหล่านี้เพื่อปรับปรุงความสามารถในการอ่านโค้ด
ปรับปรุง: แก้ไขใน ดึงคำขอ #71 และ ดึงคำขอ #77.
[N15] การใช้ uint
แทน uint256
เพื่อสนับสนุนความชัดเจน ทุกกรณีของ uint
ควรประกาศเป็น uint256
. โดยเฉพาะผู้ที่อยู่ใน for
วนซ้ำบนเส้น 98 และ 119 of OrderMixin.sol
และเส้น 16 และ 30 of PredicateHelper.sol
.
ปรับปรุง: แก้ไขใน ดึงคำขอ #70.
สรุป
พบปัญหาความรุนแรงสูง 3 ประเด็น การเปลี่ยนแปลงบางอย่างได้รับการเสนอให้เป็นไปตามแนวทางปฏิบัติที่ดีที่สุดและลดพื้นผิวการโจมตีที่อาจเกิดขึ้น
- &
- 7
- เกี่ยวกับเรา
- เข้า
- ตาม
- ลงชื่อเข้าใช้
- ข้าม
- กระทำ
- การปฏิบัติ
- เพิ่มเติม
- ที่อยู่
- ความได้เปรียบ
- ทั้งหมด
- การอนุญาต
- แล้ว
- แม้ว่า
- จำนวน
- การวิเคราะห์
- API
- เข้าใกล้
- ข้อโต้แย้ง
- รอบ
- สินทรัพย์
- สินทรัพย์
- การตรวจสอบบัญชี
- Back-end
- การเริ่มต้น
- กำลัง
- ที่ดีที่สุด
- ปฏิบัติที่ดีที่สุด
- บิต
- บอท
- สร้าง
- โทรศัพท์
- ซึ่ง
- กรณี
- ก่อให้เกิด
- chainlink
- เปลี่ยนแปลง
- การตรวจสอบ
- การตรวจสอบ
- รหัส
- ซับซ้อน
- สภาพ
- ความสับสน
- การก่อสร้าง
- มี
- สัญญา
- สัญญา
- การแก้ไข
- ค่าใช้จ่าย
- ได้
- คู่
- ผู้สร้าง
- เงินตรา
- ปัจจุบัน
- ข้อมูล
- จัดการ
- Denial of Service
- ปรับใช้
- ออกแบบ
- นักพัฒนา
- พัฒนาการ
- DID
- แตกต่าง
- ต่าง
- โดเมน
- สอง
- พลวัต
- ก่อน
- ขอบ
- ส่งเสริม
- โดยเฉพาะอย่างยิ่ง
- ETH
- เหตุการณ์
- เหตุการณ์
- ทุกอย่าง
- ตัวอย่าง
- ตลาดแลกเปลี่ยน
- ที่คาดหวัง
- ประสบการณ์
- เอาเปรียบ
- คุณสมบัติ
- สาขา
- ในที่สุด
- ชื่อจริง
- แก้ไขปัญหา
- ความยืดหยุ่น
- ไหล
- ปฏิบัติตาม
- พบ
- เต็ม
- ฟังก์ชัน
- เงิน
- อนาคต
- เกม
- GAS
- General
- ให้
- ยิ่งใหญ่
- ให้คำแนะนำ
- การจัดการ
- กัญชา
- hashing
- มี
- ช่วย
- จุดสูง
- อย่างสูง
- สรุป ความน่าเชื่อถือของ Olymp Trade?
- HTTPS
- เป็นลูกผสม
- แยกแยะ
- ส่งผลกระทบ
- การดำเนินการ
- สำคัญ
- การนำเข้า
- รวม
- รวมทั้ง
- เพิ่ม
- เพิ่มขึ้น
- ข้อมูล
- ข้อมูล
- โครงสร้างพื้นฐาน
- ข้อมูลเชิงลึก
- ความตั้งใจ
- อยากเรียนรู้
- อินเตอร์เฟซ
- ร่วมมือ
- ปัญหา
- IT
- ใหญ่
- ที่มีขนาดใหญ่
- นำ
- ชั้นนำ
- ห้องสมุด
- ถูก จำกัด
- Line
- สภาพคล่อง
- จดทะเบียน
- รายการ
- ในประเทศ
- มอง
- ค้นหา
- สำคัญ
- เครื่องชง
- การทำ
- ตลาด
- เมมพูล
- กระจก
- แบบ
- มากที่สุด
- เป็นที่นิยม
- คือ
- คุณสมบัติใหม่
- ไม่ใช่ทดแทน
- โทเค็นที่ไม่สามารถเข้าถึงได้
- เป็นทางการ
- เปิด
- การดำเนินการ
- ตัวเลือกเสริม (Option)
- คำพยากรณ์
- ใบสั่ง
- คำสั่งซื้อ
- อื่นๆ
- เจ้าของ
- แบบแผน
- ยอดนิยม
- นำเสนอ
- การป้องกัน
- ราคา
- การตั้งราคา
- ส่วนตัว
- กระบวนการ
- โครงการ
- โครงการ
- การป้องกัน
- โปรโตคอล
- ให้
- ให้
- หนังสือมอบฉันทะ
- สาธารณะ
- ประกาศ
- ซื้อ
- คุณภาพ
- ยก
- ความจริง
- ลด
- ความเชื่อมั่น
- รายงาน
- รายงาน
- กรุ
- ความต้องการ
- REST
- ผลสอบ
- รับคืน
- ทบทวน
- ความเสี่ยง
- รอบ
- วิ่ง
- SDK
- ความปลอดภัย
- บริการ
- ชุด
- ที่ใช้ร่วมกัน
- หุ้น
- เปลี่ยน
- คล้ายคลึงกัน
- ง่าย
- เล็ก
- สมาร์ท
- สัญญาสมาร์ท
- So
- ความแข็งแรง
- สแปม
- เฉพาะ
- การใช้จ่าย
- จุด
- เริ่มต้น
- สถานะ
- คำแถลง
- สหรัฐอเมริกา
- Status
- การเก็บรักษา
- สไตล์
- ส่ง
- ความสำเร็จ
- ที่ประสบความสำเร็จ
- ประสบความสำเร็จ
- สนับสนุน
- ที่สนับสนุน
- พื้นผิว
- สวิตซ์
- ระบบ
- เป้า
- ทดสอบ
- การทดสอบ
- การทดสอบ
- ตลอด
- ตลอด
- เวลา
- ร่วมกัน
- โทเค็น
- ราชสกุล
- ลู่
- การติดตาม
- การทำธุกรรม
- วางใจ
- เป็นเอกลักษณ์
- การปรับปรุง
- us
- การใช้งาน
- USD
- USDT
- ผู้ใช้
- ประโยชน์
- ความคุ้มค่า
- รายละเอียด
- ความชัดเจน
- รอ
- อะไร
- ยกเว้น
- ภายใน
- ไม่มี
- งาน
- คุ้มค่า
- เป็นศูนย์