QA עצמי

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

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

הבעיה התקיימה בפונקציה register_file היות וכתבתי את הקוד בצורה הזו:

84   int length          = strlen(pathname) +1;
85   memset(file_rec->pathname, 0, FILENAME_MAX);
86   strncpy(file_rec->pathname, pathname, length);

כמו שאפשר לראות בשורה 86, בעוד ש file_rec->pathname הוא בגודל של FILENAME_MAX, , במקום לוודא שהאורך של pathname (פרמטר שאני מקבל בפונקציה) אינו גדול יותר מ FILENAME_MAX אני מניח שמישהו יבדוק את זה כשהוא מזין לי את אורך המחרוזת, ובכך אקבל אורך מחרוזת שהוא לא גדול מ FILENAME_MAX. כמובן שזו הנחה שגוייה, ולכן אם תשתמשו בAPI הזה ללא העתקה של מחרוזת באורך המתאים, יהיה ניתן לגרום לbuffer overflow אצלי בקוד.

לכן תיקנתי את הקוד בצורה הבאה:

84   int length          = strlen(pathname) +1;
85   if (length > FILENAME_MAX) /* Making sure not to have a buffer overflow !!! */
86   {
87     length = FILENAME_MAX;
88   }
89   memset(file_rec->pathname, 0, FILENAME_MAX);
90   strncpy(file_rec->pathname, pathname, length -1);

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

חשוב להבהיר שהקוד שלי לכשעצמו לא הכיל buffer overflow היות והשימוש בפונקציה התבצע בצורה הבאה:

206       char path[FILENAME_MAX];
207       memset(&path, 0, FILENAME_MAX);
208       strncpy(path, argv[counter], FILENAME_MAX - 1);
209       if (path_exists(path))
210       {
211         if (! register_file(list, path, FLAGS))

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

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

9 מחשבות על “QA עצמי

  1. ארתיום

    עידו, מספר הערות: זה **לא** נכון לכתוב כך, הדרך הנכונה לכתוב היא:

    strncpy(file_rec->pathname, pathname, FILENAME_MAX-1);‎
    file_rec->pathname[FILENAME-1]=0;‎ // make sure the string is 0 terminated

    ולא כמו שכתבת לעיל, הפרמטר השלישי של strncpy הוא גודל buffer היעד ולא המקור… ו־strncpy לא יבצע buffer overflow במידה וגודל של pathname הוא FILENAME_MAX

    כמובן, man strncpy…

    אלא אם זאת בדיחה של 1 באפריל.

  2. ik_5 מאת

    זה ממש לא 1 באפריל.

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

    אם C היתה שפה נורמאלית, אז כל הנושא של עבודה עם מחרוזות היה מסודר, כי היא הרי לא באמת תומכת במחרוזות כמו שאתה יודע.

  3. ארתיום

    תשמע, הבעיה היא שאתה לא מסתכל נכון. להזכירך ב־Pascal גם אין מחרוזות (כן, אני מדבר על Standard Pascal).

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

    פשוט צריך לחשוב אחרת. נתחיל:

    – הרבה פעמים אתה מקצה זיכרון ואז עושה לו memset… בדיוק בשביל זה יש calloc שיותר יעיל בהקצאות גדולו.
    – לא צריך לכתוב ‎""‎ כי "" זה כבר מחרוזת שמסתיימת ב־0.
    למשל, חיפוש השם לפי wd הייתי כותב ככה:


    char *find_path_name(struct file_list *list,int wd)
    {
    struct filerecord *p;
    for(p=list->files;p;p=p->next) {
    if(p->wd==wd)
    return p->pathname;
    }
    return NULL;
    }

    נכון שזה הרבה יותר פשוט?

    בנוסף יש לך שם memory leak כי אתה צד אחד מקצה זיכרון ואז נותן למשתמש לשחרר ומצד שני אתה מחזיר "" כך שאפילו משתמש לא יכול לדעת מה לעשות.

    תשמע עידו, מצטער להגיד אבל אתה **לא** יודע C. בשביל להגיד כי השפה לא טובה או טובה צריך ללמוד אותה קודם.

    אני לא מתפלא שאתה מאוד לא אוהב C++‎ כי אתה פשוט לא מכיר אותו מספיק טוב. (גם אני הייתי שם פעם עד שלמדתי להשתמש בכוח שלו, ואז השמיים אינם הגבול).

  4. ik_5 מאת

    מוזר ש valgrind בניגוד אלייך לא חושב שיש דלפית זיכרון, כנראה שאתה יודע יותר טוב…

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

    ארתיום, C לא תומל במחרוזות. זה שיש לך מפרשים שלמים שכתובים בשפה, לא אומר שום דבר פרט לזה שאנשים אוהבים להתעלל בעצמם. גם בפסקל יש לי Null terminated string ואני משתמש בהם כשצריך. מתי זה טוב ? כאשר אני צריך לעשות מעבר אחד על כל התווים כאשר הגודל הוא גודל קבוע (כלומר בית אחד או 2 בתים וכו', אבל תמיד זה הגודל). כאשר אתה צריך לעשות פעולות מרובות על null terminated string זה כבר לא יעיל. אפילו יואל ספולסקי כתב על זה פעם, אבל עזוב אני אל רוצה שתצא מהאשליות שלך.

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

    העניין עם ++C הוא פשוט, וגם כאן זה כבר נאמר מליון פעם על ידי אנשים אחרים, השפה נפוחה כמו לא יודע מה, וכאשר אתה באמת רוצה לגרום לה להיות שימושית, אתה או צריך ספרייה מסויימת שתרסן את העודף טכנולוגיות שיש לך, או שאתה מתעלם מהטכנולוגיות ויורד לרמה מאוד נמוכה. זו הסיבה שהמציאו את ג'אווה ועוד כמה שפות, בשביל לא לחזור על אותן הטעויות.
    ב |++C אין שמים, כי אחרת גם הם היו נופלים עלייך. בפסקל אתה מסוגל לעשות כל דבר שאתה יכול לעשות בכל שפה אחרת, רק ב99% מהפעמים, הכמות אנרגייה תהיה פחותה מזו שתשקיע ב C, ו ++C.

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

    אם העולם היה מתנהל לפי מה שטוב ולא לפי אופנה, C כבר מזמן היה בקבר שלו.
    ו ++C היה אות קין לאיך לא לבנות שפת תכנות.

  5. ארתיום

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

    שוב, אני חוזר ואומר, ב־C אתה חושב אחרת. אני מסכים איתך של־C חסרים הרבה דברים: היא לא OOP, אין לה מחרוזות, אין בה ירושה, אין בה gc… אבל… אפשר לכתוב בה הרבה מאוד דברים בצורה מאוד יפה.

    פשוט כשאתה כותב ב־C אל תצפה ממנה להיות Pascal כמו שכשאני כותב ב־Java אני לא מצפה ממנה להיות C++‎ או Python.

    שוב, אם אתה כותב ב־C תכתוב כמו ב־C. זה הכל.

    מוזר ש valgrind בניגוד אלייך לא חושב שיש דלפית זיכרון

    כי הוא מחפש דליפה בהרצה ספציפית ולא את האפשרות שלה באופן כללי.

    תסתכל בקוד הזה:

    char *pathname = find_path_name(list, pevent->wd);

    free(pathname);‎

    – אם find_path_name לא תמצא את wd (שכנראה זה לא קורא) אז אתה תחזיר מחרוזת ריקה שהיא לא על ה־stack ואז כשתקרא ל־free(pathname)‎ התכנה פשוט תקרוס.
    – שנית, אם היא שום לא מוצאת את wd היא לא מוחקת זיכרון if(strlen(name)>0) return name;‎
    אבל אם הוא כן שווה ל־0 אתה לא דואג לשחרר אותו.
    בנוסף, strlen(name)>0 זה פשוט לא יעיל, תכתוב name[0]!=0 או ‎ *name!=0זה יותר נכון ויעיל.

    תשמע, בקטע הקוד שלך יש לך שתי שגיאות חמורות:

    1. אתה עלול לגרום ל־memory leak
    2. אתה עלול לגרום לקריסה של תכנה.

    בקיצור — אתה באמת חייב לחשוב אחרת כדי לכתוב ב־C. זאת המציאות.

  6. ik_5 מאת

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

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

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

    מה דעתך על האתגר ?

  7. ארתיום

    כלומר יוכיח שיש memory leak ויגרום לקריסה של התוכנית

    תשמע, זה לו זולג כי תמיד wd שאתה מקבל הוא חוקי.

    בואו נעשה תרגיל קטן: תבנה ספריה foo: קרי mkdir foo
    תריץ ‎./notify foo

    כאשר notify רצה, תמחק את foo ועכשיו תנסה לעצור תכנה שלך עם ctl-C… רגע מה קרה? היא לא עוצרת, משמעה היא גם זולגת 😉

להשאיר תגובה

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

הלוגו של WordPress.com

אתה מגיב באמצעות חשבון WordPress.com שלך. לצאת מהמערכת / לשנות )

תמונת Twitter

אתה מגיב באמצעות חשבון Twitter שלך. לצאת מהמערכת / לשנות )

תמונת Facebook

אתה מגיב באמצעות חשבון Facebook שלך. לצאת מהמערכת / לשנות )

תמונת גוגל פלוס

אתה מגיב באמצעות חשבון Google+ שלך. לצאת מהמערכת / לשנות )

מתחבר ל-%s