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

1. פירצת אבטחה שאי אפשר לנצל

הפונקציה הבאה ב Rails מיועדת לשמירת קובץ שמשתמש מעלה:

  def save_uploaded_image
    uploaded_io = params[:item][:image]
    filepath = Rails.root.join('public', 'uploads', uploaded_io.original_filename)
    raise 'Faile Exists' if File.exist?(filepath)

    File.open(filepath, 'wb') do |file|
      file.write(uploaded_io.read)
    end
    @item.image_path = "/uploads/#{uploaded_io.original_filename}"
  end

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

ולמי שתוהה מה נזק יכול להיגרם מזה כי הרי ממילא אי אפשר לדרוס קבצים, נספר שזה תלוי מערכת ותלוי מקרה אבל כמעט בכל מערכת העלאת קבצים לכל מקום בשרת הופכת מהר מאוד להשתלטות על השרת (לדוגמא: מעלים קובץ php לתיקיה שהשרת מפעיל ממנה קבצי php וגולשים אליו. או מעלים קובץ רובי לתיקיה ממנה השרת טוען ומריץ קבצים אוטומטית).

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

2. פורטביליות: אפשרות העברת קוד פגיע לפריימוורק אחר

ככל שמערכת גדלה כך חלקים בקוד מועתקים למקומות אחרים, כולל בין Frameworks. מתכנתים אוהבים לעשות Copy/Paste ואם יש חלק בקוד שכולל לוגיקה מורכבת שקשורה למערכת שלכם מישהו שעובד אתכם עלול להעתיק אותה לסביבה או לשפה אחרת. מה אם באותה שפה אחרת ההגנה אינה אוטומטית?

שימו לב לדוגמא לקוד מקביל ב PHP:

<?php
// In PHP versions earlier than 4.1.0, $HTTP_POST_FILES should be used instead
// of $_FILES.

$uploaddir = '/var/www/uploads/';
$uploadfile = $uploaddir . basename($_FILES['userfile']['name']);

echo '<pre>';
if (move_uploaded_file($_FILES['userfile']['tmp_name'], $uploadfile)) {
    echo "File is valid, and was successfully uploaded.\n";
} else {
    echo "Possible file upload attack!\n";
}

echo 'Here is some more debugging info:';
print_r($_FILES);

print "</pre>";
?>

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

3. ניקוי קלט עוזר לנו להבין לאיזה ערכים אנחנו מצפים

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

בדוגמת העלאת הקבצים שלנו, אולי נרצה בנוסף להפעלת basename גם לוודא שאפשר להעלות רק תמונות או רק קבצי PDF. הסתמכות על קוד חיצוני שינקה עבורנו את הקלט לא נותנת את ההזדמנות לחשוב על זה, וכך אולי באמת אנחנו חסינים מ Directory Traversal Attack אבל נשארים פגיעים למתקפות ספציפיות למקרה שלנו.

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

4. ההגנה החיצונית לא מכסה את כל המקרים

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

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

5. הרגל הופך לטבע

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

  def save_uploaded_image
    uploaded_io = params[:item][:image]
    filepath = Rails.root.join('public', 'uploads', uploaded_io.original_filename)

    filepath = File.basename(filepath)
    raise 'Invalid Filename' unless filepath =~ /^[a-z_0-9]+\.(jpg|jpeg|png)$/

    raise 'Faile Exists' if File.exist?(filepath)

    File.open(filepath, 'wb') do |file|
      file.write(uploaded_io.read)
    end
    # original_filename =~ /^[a-z]+\.jpg$/
    @item.image_path = "/uploads/#{uploaded_io.original_filename}"
  end