อธิบายการรีวิวโค้ดด้านความปลอดภัย: ผู้รีวิวมองหาอะไรใน diff และเพราะเหตุใด

การทบทวนโค้ดด้านความปลอดภัยคือการอ่านซอร์สโค้ดอย่างตั้งใจโดยมีจุดประสงค์เฉพาะเพื่อค้นหาวิธีที่โค้ดนั้นสามารถถูกทำให้ทำสิ่งที่ผู้เขียนไม่ได้ตั้งใจ คำจำกัดความนี้ฟังดูชัดเจนจนกว่าคุณจะเปรียบเทียบกับสิ่งที่ทีมส่วนใหญ่ทำจริง ซึ่งคือการทบทวนเชิงฟังก์ชันโดยมีรายการตรวจสอบด้านความปลอดภัยวางอยู่ใกล้ ๆ OWASP Secure Code Review Cheat Sheet กำหนดเส้นแบ่งไว้อย่างแม่นยำ: การทบทวนด้านความปลอดภัยแตกต่างจากการทบทวนเชิงฟังก์ชันตรงที่มุ่งเป้าไปที่การตรวจสอบอินพุต การพิสูจน์ตัวตน การอนุญาต การนำวิทยาการเข้ารหัสลับไปใช้งาน และเวกเตอร์การโจมตี แทนที่จะมุ่งที่ความถูกต้อง diff ที่ทำงานได้อย่างสมบูรณ์แบบก็ยังสามารถมอบเส้นทางให้ผู้โจมตีเข้าสู่ระบบได้ และมีเพียงหนึ่งในสองการทบทวนนั้นเท่านั้นที่กำลังมองหาสิ่งนี้

สิ่งที่ทำให้หัวข้อนี้น่าควรค่าแก่การเขียนบทความตอนนี้แทนที่จะเป็นในปี 2020 คือแบบจำลองภาษาสามารถอ่าน diff ได้อย่าง competent ในต้นทุนต่ำและต่อเนื่อง ซึ่งเปลี่ยนเศรษฐศาสตร์ของการรีวิวโดยไม่เปลี่ยนแม้แต่มิติเดียวที่ถูกรีวิว เช็กลิสต์มีอายุเก่ากว่าตัวแบบและยังไม่เปลี่ยนแปลง สิ่งที่เปลี่ยนไปคือความถี่ที่คุณสามารถจ่ายเพื่อรันมันได้

ผู้ตรวจทานอ่านเพื่อหาอะไรกันแน่

ชีตสรุปของ OWASP จัดระเบียบงานออกเป็นหกด้าน และสิ่งที่มีประโยชน์เกี่ยวกับรายการนี้คือแต่ละด้านเป็นคำถามมากกว่าหมวดหมู่ หมวดหมู่ชวนให้ติ๊กถูกตามรายการ ส่วนคำถามนำไปสู่ข้อค้นพบ

อินพุตและการแทรก คำถามไม่ใช่ว่าโค้ดมี sanitizer หรือไม่ แต่คือมีสิ่งใดบ้างที่ไปถึงตัวแปลคำสั่ง — เชลล์, SQL, เทมเพลตเอนจิน, ตัวประเมินนิพจน์ — และข้อกำหนดนั้นควรถูกบังคับใช้ที่เลเยอร์ใด diff ที่เพิ่มคิวรีแบบกำหนดพารามิเตอร์เป็นสิ่งดี แต่ diff ที่เพิ่มคิวรีแบบกำหนดพารามิเตอร์เข้าไปในฟังก์ชันที่ผู้เรียกอื่นส่งสตริงดิบเข้ามา คือ diff ที่ย้ายปัญหาไปที่อื่น

การยืนยันตัวตนและเซสชัน ใครคือผู้เรียก และโค้ดยืนยันเรื่องนั้นได้อย่างไร บั๊กที่เกิดขึ้นซ้ำไม่ใช่การตรวจสอบรหัสผ่านที่พัง แต่เป็นเซสชันที่ยังคงอยู่รอดหลังการเปลี่ยนสิทธิ์ — การยกระดับสิทธิ์เกิดขึ้น โทเคนยังคงถือ claims เดิมไว้ และผ่านไปหนึ่งชั่วโมงสิทธิ์เหล่านั้นก็ยังอยู่

การอนุญาตการตรวจสอบระดับออบเจกต์ ทุกครั้ง และในทุกเส้นทาง สิ่งที่ต้องสังเกตเป็นพิเศษใน diff คือเส้นทางใหม่ที่ไม่ได้รับการสืบทอด guard ที่เส้นทางพี่น้องมีอยู่ เพราะ middleware ถูกขึ้นทะเบียนแยกตามแต่ละ router และเส้นทางที่ถูกเพิ่มภายใต้ router อื่นก็จะไม่ถูกครอบคลุมเลย

วิทยาการเข้ารหัสลับ ชนิดของ primitive โหมด ความยาวคีย์ และตำแหน่งที่เก็บคีย์ คำถามสองข้อที่จับประเด็นการค้นพบจริงได้เกือบทั้งหมดคือ ใครก็ตามที่อ่านโค้ดเบสนี้ได้จะอ่านคีย์ได้ด้วยหรือไม่ และการเปรียบเทียบนี้เป็นแบบที่ต้องไม่รั่วข้อมูลด้านเวลาหรือเปล่า

ตรรกะทางธุรกิจ ไม่ว่าลำดับจะถูกจัดเรียงใหม่ เล่นซ้ำ หรือถูกขับเคลื่อนด้วยค่าที่ไคลเอนต์เลือก ราคา จำนวน บทบาท หรือการเปลี่ยนสถานะที่ส่งมาจากไคลเอนต์และไม่ได้ถูกคำนวณใหม่ฝั่งเซิร์ฟเวอร์ ถือเป็นข้อค้นพบไม่ว่าจะถูกตรวจสอบอย่างไร

การกำหนดค่าและข้อมูลลับ สิ่งที่ diff นำส่งออกไปซึ่งไม่ควรอยู่ในรีพอซิทอรีตั้งแต่แรก และค่าเริ่มต้นนั้นปลอดภัยหรือไม่ ณ ที่ที่โค้ดรันจริง แฟล็กดีบักที่ตั้งค่าเริ่มต้นเป็น true ถือเป็นข้อค้นพบเกี่ยวกับโปรดักชัน ไม่ใช่เกี่ยวกับตัวแฟล็กเอง

การรีวิวแบบ Baseline เทียบกับการรีวิวแบบ diff

OWASP แยกออกเป็นสองโหมด และการนำสองโหมดนี้มาปะปนกันมักเป็นต้นเหตุของทั้งความพยายามที่สูญเปล่าและความมั่นใจที่ผิดพลาด การตรวจทานแบบ baselineจะตรวจสอบฐานโค้ดอย่างครอบคลุม และเป็นเครื่องมือที่เหมาะสมสำหรับแอปพลิเคชันใหม่หรือการเปิดตัวเวอร์ชันสำคัญ การนำระบบเดิมเข้าสู่กระบวนการ การตอบสนองข้อกำหนดด้านการปฏิบัติตามกฎระเบียบ หรือการวิเคราะห์หลังเกิดเหตุหลังจากมีบางอย่างผิดพลาดไปแล้ว การตรวจทานโดยอิง diffจะพิจารณาเฉพาะสิ่งที่เปลี่ยนแปลง และเป็นเครื่องมือที่เหมาะสมสำหรับ pull request, commit, การทำฟีเจอร์ให้เสร็จสมบูรณ์ และการตรวจสอบความถูกต้องอย่างต่อเนื่อง

ความแตกต่างนี้สำคัญ เพราะทั้งสองวิธีให้ผลลัพธ์งานที่แตกต่างกัน การรีวิวแบบ baseline จบลงด้วย backlog ส่วนการรีวิวแบบ diff จบลงด้วยการตัดสินใจ merge การรันแบบแรกอย่างต่อเนื่องมีต้นทุนสูงจนรับไม่ไหว และการรันแบบที่สองบนโค้ดเบสที่ไม่เคยผ่านแบบแรกเลย คือวิธีที่ทีมสะสมความรู้สึกผิด ๆ ว่าตนครอบคลุมแล้ว: ทุกการเปลี่ยนแปลงผ่านการรีวิว ขณะที่การต่อสตริง SQL ที่มีอยู่เดิมยังคงอยู่เฉย ๆ ไม่ถูกแตะต้อง

เทคนิค และสิ่งที่แต่ละเทคนิคจับได้

OWASP ระบุเทคนิคการตรวจสอบสี่ประเภท และเทคนิคเหล่านี้จำแนกตามหลักฐานที่ผู้ตรวจสอบติดตาม ไม่ใช่ตามระดับความละเอียดถี่ถ้วน การวิเคราะห์รูปแบบโค้ด ติดตามรูปแบบที่คุ้นเคย เช่น การเรียกฟังก์ชันอันตราย การดีซีเรียลไลซ์ข้อมูลที่เชื่อถือไม่ได้ และการตรวจสอบความยาวที่ขาดหายไป ซึ่งรวดเร็วและเป็นสิ่งที่เครื่องมือแบบดีเทอร์มินิสติกทำได้ดี การวิเคราะห์การไหลของข้อมูล ติดตามค่าจากจุดที่มันเข้ามาจนถึงจุดที่ถูกใช้ ซึ่งเป็นวิธีเดียวที่จะตอบได้ว่าตัวทำความสะอาดข้อมูลที่ใช้เมื่อสามฟังก์ชันก่อนยังใช้ได้ผล ณ จุดนี้หรือไม่ การตรวจสอบตามภัยคุกคาม เริ่มจากความสามารถของผู้โจมตีแล้วไล่ไปข้างหน้า ซึ่งเป็นวิธีที่คุณจะจับช่องว่างระหว่างองค์ประกอบที่ถูกต้องแต่ละตัวได้ การตรวจสอบตรรกะทางธุรกิจ อ่านเวิร์กโฟลว์มากกว่าอ่านทีละบรรทัด โดยถามว่าลำดับการดำเนินการที่ชอบด้วยกฎหมายแบบใดก่อให้เกิดสถานะที่ไม่ชอบด้วยกฎหมาย

ในสี่ข้อนั้น มีเพียงข้อแรกเท่านั้นที่ scanner ให้ผลได้ดี และมีเพียงข้อแรกกับข้อที่สองที่การให้โมเดลไล่ตรวจแบบไม่มีแนวทางชี้นำให้ผลได้ดี นั่นไม่ใช่ข้อโต้แย้งต่อการใช้โมเดล แต่เป็นข้อโต้แย้งให้บอกผู้ตรวจสอบว่าคุณต้องการเทคนิคใด เพราะพรอมต์ที่ขอให้หา "ปัญหาด้านความปลอดภัย" จะให้ผลเป็นการวิเคราะห์ตามรูปแบบซ้ำ ๆ แล้วหยุดเท่านั้น

AI เปลี่ยนแปลงอะไร และไม่ได้เปลี่ยนแปลงอะไร

งานวิจัยเกี่ยวกับโค้ดที่ AI เขียนขึ้นทำให้คอขวดในการตรวจทานชัดเจนยิ่งขึ้น แทนที่จะเป็นตัวก่อให้เกิดคอขวดนั้น การศึกษาของ NYU เกี่ยวกับข้อเสนอแนะของ GitHub Copilot ซึ่งตีพิมพ์ใน IEEE S&P เมื่อปี 2022 พบว่าประมาณ 40% ของโค้ดตัวอย่างที่สร้างขึ้นมีรูปแบบที่ใช้โจมตีได้ ครอบคลุม 18 หมวดหมู่ CWE งานวิจัยของ Stanford เกี่ยวกับพฤติกรรมนักพัฒนา ซึ่งตีพิมพ์ใน ACM CCS เมื่อปี 2023 พบผลกระทบที่สำคัญกว่า: นักพัฒนาที่ใช้ผู้ช่วย AI มีความมั่นใจว่าโค้ดของตนปลอดภัยมากขึ้น ในขณะที่สร้างโค้ดที่ปลอดภัยน้อยลงอย่างวัดได้ ปริมาณผลลัพธ์ที่มากขึ้นผนวกกับความมั่นใจที่เพิ่มขึ้นกลายเป็นปัญหาในการตรวจทาน

โมเดลภาษาในฐานะผู้ตรวจสอบเปลี่ยนตัวแปรหนึ่งในสมการนั้น — ทรูพุต มันสามารถอ่าน pull request ทุกอันได้ ด้วยต้นทุนต่อโทเค็นแทนที่จะเป็นต่อที่นั่ง และสามารถสั่งให้ตอบคำถามทั้งหกข้อข้างต้นอย่างชัดเจนได้ มันไม่ได้เปลี่ยนคำถาม และไม่ได้มีคุณสมบัติสองประการที่ทำให้การตรวจสอบน่าเชื่อถือ ได้แก่ ความแน่นอนของผลลัพธ์ และความสามารถในการรันโค้ด diff เดียวกันที่ถูกตรวจสองครั้งอาจให้ข้อค้นพบที่ต่างกัน ระดับความรุนแรงเปลี่ยนไประหว่างการรันแต่ละครั้ง และผู้ตรวจสอบไม่สามารถพิสูจน์ได้ว่าเส้นทางที่ถูกระบุว่าเป็นปัญหานั้นเข้าถึงได้จริง

OrcaCode Reviewคือรูปแบบที่ถูกทำให้เป็นผลิตภัณฑ์ของแนวคิดนั้น และน่าพิจารณาอย่างใกล้ชิด เพราะการเลือกการออกแบบเผยให้เห็นโหมดความล้มเหลว การรีวิวที่แยกเคสที่ล้มเหลวออกมาเป็นความคิดเห็นเฉพาะเจาะจงที่ทำซ้ำได้ — คีย์นี้ยังคงแปรผันตามเวลา ดังนั้นการเรียกเก็บเงินที่ลองใหม่จะสร้างคีย์ใหม่และอาจเรียกเก็บซ้ำได้ นี่คือการเปลี่ยนแปลงเพียงบรรทัดเดียว — กำลังทำหน้าที่ที่ผู้รีวิวทำ การรีวิวที่รายงานว่า "ปัญหาความปลอดภัยที่อาจเกิดขึ้น" โดยไม่มีการทำซ้ำ คือการวิเคราะห์รูปแบบที่มีความแม่นยำต่ำกว่าตัวจับรูปแบบ และเอนจินกฎและเกตที่เผยแพร่ภายใต้สัญญาอนุญาตแบบเปิดกำลังสร้างข้ออ้างโดยนัยที่น่าตรวจสอบ: ว่าการตัดสินของผู้รีวิวก็ควรตรวจสอบได้เช่นกัน

การจัดวางแบบสองชั้นที่ได้ผล

เมื่อมีตัวสแกนแบบดีเทอร์มินิสติกและตัวรีวิวแบบความน่าจะเป็น รูปแบบที่ผ่านการใช้งานจริงคือการมอบงานที่แต่ละตัวถนัดจริง ๆ ให้กับมัน ตัวสแกนทำงานทุกครั้งที่พุช: ใช้เวลาไม่กี่วินาที ไม่ใช้โมเดล ไม่มีความแปรปรวน จับ secrets การเรียก API ที่อันตราย การดีซีเรียลไลเซชันที่ไม่ปลอดภัย และการนำเข้าที่ถูกแต่งขึ้นแบบหลอน ตัวรีวิวทำงานทุกครั้งที่ pull request: ใช้เวลาหลายนาที ขับเคลื่อนด้วยโมเดล จับตรรกะที่แฝงอยู่ระหว่างบรรทัด — ว่าการตรวจสอบสิทธิ์ทำงานก่อนการดึงข้อมูลหรือไม่ ว่าคีย์ idempotency ผูกกับค่าที่เสถียรหรือไม่ และว่าเส้นทางใหม่สืบทอด guard มาหรือไม่

จากนั้นคือขั้นตอนที่ผู้คนมักข้ามไป: ด่านตรวจ ข้อค้นพบที่ไม่บล็อกการ merge ก็เป็นเพียงการแจ้งเตือน และการแจ้งเตือนจะถูกอ่านในวันศุกร์ ด่านตรวจต้องแคบและต้องเด็ดขาด — ข้อค้นพบระดับวิกฤตต้องบล็อก ส่วนอย่างอื่นให้คอมเมนต์ — และการยกเว้นต้องถูกเขียนไว้ เพราะคุณค่าของด่านตรวจไม่ใช่การหยุดข้อค้นพบแรก แต่คือการทำให้การข้ามทุกครั้งมองเห็นได้

ตรวจสอบการเปลี่ยนแปลง ไม่ใช่โค้ดเบส

วินัยภาคปฏิบัติสำหรับการรีวิว diff คือลำดับขั้นตอนสั้น ๆ และคุณค่าของมันอยู่ที่ว่าสามารถสอนได้ภายในบ่ายเดียว อ่าน diff เพื่อดูว่ามันเพิ่มอะไรให้พื้นผิวการโจมตี แทนที่จะดูว่ามันเปลี่ยนแปลงอะไร สำหรับจุดเข้าใหม่แต่ละจุด ให้ตามข้อมูลไปหนึ่งช่วงเลยจุดที่ diff สิ้นสุดลง — ช่องโหว่มักอยู่ในฟังก์ชันที่มันเรียกใช้ ซึ่งผู้เขียนไม่ได้อ่าน สำหรับเส้นทางใหม่แต่ละเส้นทาง ให้ตรวจสอบการลงทะเบียน guard สำหรับ dependency ใหม่แต่ละรายการ ให้ตรวจสอบว่ามีอยู่จริงหรือไม่ และมีคนอื่นเผยแพร่ชื่อนั้นไปแล้วหรือไม่ สำหรับค่าคอนฟิกูเรชันใหม่แต่ละค่า ให้ถามว่าค่าเริ่มต้นของมันคืออะไร ณ ที่ที่จะนำไปใช้งานจริง

ไม่มีสิ่งใดในนั้นที่ต้องรู้ภาษาดี ซึ่งเป็นเหตุผลว่าทำไมวิธีนี้จึงขยายไปยังผู้ตรวจทานที่ไม่ใช่ผู้เชี่ยวชาญด้านความปลอดภัยได้ แต่ต้องให้ผู้ตรวจทานเห็น diff ไม่ใช่เห็นไฟล์ เพราะวิธีการทั้งหมดขึ้นอยู่กับการรู้ว่าอะไรคือของใหม่

และข้อจำกัดที่ตรงไปตรงมาคือ การรีวิว diff ไม่สามารถพบข้อบกพร่องที่มีอยู่ก่อน diff ได้ หากแอปพลิเคชันไม่เคยผ่านการรีวิว baseline งานด้านความปลอดภัยที่ให้มูลค่าสูงสุดที่ทำได้ไม่ใช่การมีผู้รีวิวที่ดีกว่า — แต่คือการตรวจทานโค้ดเบสอย่างตั้งใจหนึ่งรอบ ในพื้นที่ที่เกี่ยวข้องกับเงิน ข้อมูลระบุตัวตน และการเข้าถึงไฟล์ โดยใช้คำถามทั้งหกข้อข้างต้น หลังจากนั้น การรีวิว diff ทุกครั้งต่อจากนี้คือการปกป้องสิ่งที่เคยถูกตรวจสอบแล้วครั้งหนึ่ง ก่อนหน้านั้น การรีวิว diff ทุกครั้งคือการปกป้องสิ่งที่ไม่รู้จัก