סקירת קוד אבטחה מוסברת: מה סוקר מחפש בדיף, ולמה

סקירת אבטחה של קוד היא קריאה מכוונת של קוד מקור למטרה ספציפית: למצוא את הדרכים שבהן אפשר לגרום לו לעשות משהו שהכותב שלו לא התכוון לו. ההגדרה הזו נשמעת מובנת מאליה עד שמשווים אותה למה שרוב הצוותים למעשה מריצים — סקירה פונקציונלית עם רשימת בדיקות אבטחה בהישג יד. ה-OWASP Secure Code Review Cheat Sheet משרטט את הגבול במדויק: סקירת אבטחה נבדלת מסקירה פונקציונלית בכך שהיא מתמקדת בוולידציה של קלט, אימות, הרשאה, מימוש קריפטוגרפי ווקטורי תקיפה, ולא בנכונות. diff שעובד בצורה מושלמת עדיין יכול לתת לתוקף נתיב לתוך המערכת, ורק אחת משתי הסקירות הללו מחפשת אותו.

מה שהופך את הנושא לראוי למאמר עכשיו ולא ב-2020 הוא שמודל שפה יכול לקרוא diff ביכולת טובה, בזול ובאופן רציף — מה שמשנה את הכלכלה של הסקירה בלי לשנות אף אחד מהממדים הנסקרים. רשימת הבדיקה ותיקה יותר מהמודלים ולא זזה. מה שזז הוא באיזו תדירות אתה יכול להרשות לעצמך להריץ אותה.

מה שסוקר באמת מחפש בקריאה

דף הצ'יט של OWASP מארגן את העבודה לשישה תחומים, והדבר השימושי ברשימה הוא שכל תחום הוא שאלה ולא קטגוריה. קטגוריות מזמינות סימון וי; שאלות מפיקות ממצאים.

קלט והזרקה. השאלה אינה אם לקוד יש מסנן; אלא אם משהו מגיע למפרש — מעטפת, SQL, מנוע תבניות, מעריך ביטויים — ובאיזו שכבה אמורה להיאכף הדרישה הזו. דיף שמוסיף שאילתה עם פרמטרים הוא טוב; דיף שמוסיף שאילתה עם פרמטרים לתוך פונקציה שקוראיה האחרים מעבירים לה מחרוזות גולמיות הוא דיף שהעביר את הבעיה.

אימות וסשן. מי הקורא, וכיצד הקוד ביסס זאת? הבאג החוזר אינו בדיקת סיסמה לקויה, אלא סשן ששורד שינוי הרשאות — ההרשאה מוגברת, הטוקן ממשיך להחזיק בטענותיו הישנות, וכעבור שעה ההרשאות עדיין קיימות.

הרשאה. בדיקות ברמת האובייקט, בכל פעם, בכל נתיב. הדבר הספציפי שיש לחפש ב־diff הוא נתיב חדש שאינו יורש את מנגנון ההגנה (guard) שיש לנתיבים האחים שלו, מפני ש־middleware נרשם לפי router, ונתיב שנוסף תחת router אחר פשוט לא יכוסה.

קריפטוגרפיה. פרימיטיב, מצב, אורך מפתח, והיכן המפתח נמצא. שתי שאלות תופסות את רוב הממצאים האמיתיים: האם מי שיכול לקרוא את בסיס הקוד הזה יכול גם לקרוא את המפתח, והאם ההשוואה הזו היא כזו שאסור שתדליף תזמון.

לוגיקה עסקית. האם ניתן לסדר מחדש את הרצף, להריץ אותו מחדש, או להניע אותו באמצעות ערכים שהלקוח בחר. מחיר, כמות, תפקיד או מעבר מצב שמגיע מהלקוח ואינו נגזר מחדש בצד השרת מהווה ממצא, ללא קשר לאופן שבו הוא מאומת.

תצורה וסודות. מה שהדיף מכניס שלא היה אמור להימצא במאגר מעולם, והאם ברירות המחדל בטוחות במקום שבו הקוד באמת רץ. דגל ניפוי שגיאה שברירת המחדל שלו היא true הוא ממצא על סביבת הייצור, לא על הדגל.

סקירת בסיס לעומת סקירת הבדלים

OWASP מפריד בין שני מצבים, וערבוב ביניהם הוא מקור נפוץ גם לבזבוז מאמץ וגם לביטחון שווא. סקירת בסיס בוחנת את בסיס הקוד באופן מקיף, והיא הכלי הנכון עבור אפליקציה חדשה או מהדורה גדולה, קליטת מערכת מדור קודם, עמידה בדרישת תאימות, או ביצוע ניתוח לאחר תקרית לאחר שמשהו כבר השתבש. סקירה מבוססת diff מסתכלת רק על מה שהשתנה, והיא הכלי הנכון עבור בקשות משיכה, קומיטים, השלמת תכונה ואימות מתמשך.

ההבחנה חשובה משום ששתי הסקירות מפיקות עבודה שונה. סקירת בסיס מסתיימת בצבר משימות; סקירת דיף מסתיימת בהחלטת מיזוג. הרצה מתמשכת של הראשונה יקרה מדי מכדי להרשות לעצמה, והרצת השנייה על בסיס קוד שמעולם לא עבר את הראשונה היא הדרך שבה צוותים צוברים תחושת כיסוי כוזבת: כל שינוי עובר סקירה, ושרשור ה-SQL שהיה קיים מלכתחילה נותר שם בלי שנוגעים בו.

טכניקות, ומה כל אחת מהן מזהה

OWASP מפרט ארבע טכניקות סקירה, והן נבדלות זו מזו לפי הראיות שהסוקר עוקב אחריהן ולא לפי מידת היסודיות שלהן. ניתוח דפוסי קוד עוקב אחר צורות מוכרות: קריאת הפונקציה המסוכנת, דסריאליזציה של קלט לא מהימן, בדיקת האורך החסרה. הוא מהיר וזה בדיוק מה שכלי דטרמיניסטי עושה היטב. ניתוח זרימת נתונים עוקב אחר ערך מהמקום שבו הוא נכנס ועד המקום שבו הוא משמש, וזו הדרך היחידה לענות האם המסנן שהופעל שלוש פונקציות קודם עדיין חל כאן. סקירה מבוססת איומים מתחילה מהיכולת של התוקף ומתקדמת קדימה, וכך תופסים את הפער בין שני רכיבים שכל אחד מהם נכון בנפרד. סקירת לוגיקה עסקית קוראת את זרימת העבודה ולא את השורות, ושואלת איזה רצף של פעולות לגיטימיות יוצר מצב לא לגיטימי.

רק הראשון מבין ארבעת אלה זוכה לשירות טוב מסורק, ורק הראשון והשני זוכים לשירות טוב ממעבר מודל ללא הכוונה. זו אינה טענה נגד מודלים; זו טענה בעד לומר למבקר איזו טכניקה אתה רוצה, משום שהנחיה שמבקשת "בעיות אבטחה" תייצר באופן אמין ניתוח דפוסים ותעצור.

מה שבינה מלאכותית שינתה — ומה שהיא לא שינתה

המחקר על קוד שנכתב על ידי בינה מלאכותית החריף את צוואר הבקבוק בביקורת הקוד במקום ליצור אותו. מחקר NYU על הצעות GitHub Copilot, שפורסם ב-IEEE S&P ב-2022, מצא שכ-40% מהקטעים שנוצרו הכילו דפוס שניתן לנצל לרעה ב-18 קטגוריות CWE. עבודת סטנפורד על התנהגות מפתחים, שפורסמה ב-ACM CCS ב-2023, מצאה את ההשפעה המשמעותית יותר: מפתחים שהשתמשו בעוזרים היו בטוחים יותר שהקוד שלהם מאובטח, בעוד שיצרו קוד מאובטח פחות באופן מדיד. יותר תוצרים יחד עם יותר ביטחון הם בעיית ביקורת.

מודל שפה כמבקר משנה משתנה אחד במשוואה ההיא — תפוקה. הוא יכול לקרוא כל pull request, בעלות לפי טוקן ולא לפי מושב, ואפשר לשאול אותו את שש השאלות שלעיל במפורש. הוא אינו משנה את השאלות, ואינו רוכש את שתי התכונות שהופכות ביקורת לאמינה: דטרמיניזם והיכולת להריץ את הקוד. אותו diff שנבדק פעמיים יכול להניב ממצאים שונים, החומרות משתנות בין הרצות, והמבקר אינו יכול להוכיח שנתיב שסומן הוא בר-הגעה.

OrcaCode Reviewהוא הצורה הממוצרת של הרעיון הזה, וכדאי לבחון אותו מקרוב משום שבחירות העיצוב חושפות את אופני הכשל. סקירה שמבודדת את מקרה הכשל כהערה ספציפית וניתנת לשחזור — המפתח הזה עדיין משתנה בזמן, כך שחיוב חוזר מייצר מפתח חדש ועלול לחייב פעמיים, הנה השינוי בשורה אחת — עושה את העבודה שסוקר עושה. סקירה שמדווחת על "בעיית אבטחה פוטנציאלית" בלי שחזור עושה ניתוח דפוסים בדיוק נמוך יותר מזה של מזהה התבניות. ומנוע כללים ושערים שמפורסם תחת רישיון פתוח מציג טענה משתמעת שכדאי לבדוק: שגם שיקול הדעת של הסוקר צריך להיות ניתן לבחינה.

המערך הדו-שכבתי שעובד

בהינתן סורק דטרמיניסטי ומבקר הסתברותי, ההסדר שמחזיק מעמד בפועל הוא לתת לכל אחד את העבודה שהוא באמת טוב בה. הסורק רץ בכל push: שניות, בלי מודל, בלי שונות, ותופס סודות, קריאות API מסוכנות, דסריאליזציה לא בטוחה וייבואים הזייתיים. המבקר רץ בכל pull request: דקות, מגובה במודל, ותופס את ההיגיון שבין השורות — אם בדיקת ההרשאה רצה לפני השליפה, אם מפתח האידמפוטנטיות קשור לערך יציב, ואם הנתיב החדש ירש את השומר.

ואז השכבה שאנשים מדלגים עליה: השער. ממצא שלא חוסם מיזוג הוא התראה, והתראות נקראות בימי שישי. השער חייב להיות צר וחייב להיות מוחלט — ממצאים קריטיים חוסמים, כל השאר רק מעירים — והפטור חייב להיות מתועד, כי הערך של שער אינו בכך שהוא עוצר את הממצא הראשון אלא בכך שהוא הופך כל עקיפה לגלויה.

בוחנים את השינוי, לא את בסיס הקוד

המשמעת המעשית של סקירת diff היא רצף קצר, וערכה הוא בכך שאפשר ללמד אותה במהלך אחר צהריים אחד. קרא את ה-diff כדי לראות מה הוא מוסיף למשטח התקיפה ולא כדי לראות מה הוא משנה. עבור כל נקודת כניסה חדשה, עקוב אחרי הקלט צעד אחד מעבר למקום שבו ה-diff מסתיים — הפגיעות נמצאת בדרך כלל בפונקציה שהוא קורא לה, שאותה הכותב לא קרא. עבור כל נתיב חדש, בדוק את רישום ה-guard. עבור כל תלות חדשה, בדוק אם היא קיימת ואם מישהו אחר פרסם את השם. עבור כל ערך תצורה חדש, שאל מהי ברירת המחדל שלו בסביבה שבה הוא ייפרס בפועל.

שום דבר מזה לא דורש לדעת את השפה היטב, וזו הסיבה שזה מתרחב לסוקרים שאינם מומחי אבטחה. זה כן דורש שהסוקר יראה את ה-diff ולא את הקובץ, משום שכל השיטה תלויה בידיעה מה חדש.

והגבול הכנה: סקירת diff לא יכולה למצוא את הפגם שהיה שם לפני ה-diff. אם היישום מעולם לא עבר סקירת בסיס, העבודה הביטחונית בעלת הערך הגבוה ביותר הזמינה אינה סוקר טוב יותר — אלא מעבר מכוון אחד על בסיס הקוד, באזורים שנוגעים לכסף, לזהות ולגישה לקבצים, עם שש השאלות שלמעלה. אחרי זה, כל סקירת diff עוקבת מגנה על משהו שכבר נבדק פעם אחת. לפני זה, כל סקירת diff מגנה על משהו לא ידוע.