ห้ามใช้ `strcpy` เช่นกัน

 (daniel.haxx.se)
11 คะแนน โดย GN⁺ 2026-01-01 | 5 ความคิดเห็น | แชร์ทาง WhatsApp
  • โปรเจกต์ cURL หลังจากเคยถอด strncpy() ออกไปก่อนหน้านี้ ตอนนี้ได้ สั่งห้าม strcpy() ในโค้ดเบสทั้งหมด แล้ว
  • แม้ strcpy() จะมี API ที่เรียบง่าย แต่ก็มี ความเสี่ยงที่การตรวจสอบขนาดบัฟเฟอร์จะแยกออกจากกัน ทำให้ไม่ปลอดภัยสำหรับการบำรุงรักษาระยะยาว
  • เพื่อใช้แทน จึงมีการนำฟังก์ชันใหม่ชื่อ curlx_strcopy() เข้ามา โดยรับทั้งขนาดบัฟเฟอร์ปลายทางและความยาวสตริงเป็นอาร์กิวเมนต์ เพื่อ ตรวจสอบก่อนว่าสามารถคัดลอกได้หรือไม่แล้วจึงทำงาน
  • ฟังก์ชันนี้ใช้ memcpy() ภายใน และ รับประกันการจัดการอักขระปิดท้ายแบบ null
  • การเปลี่ยนแปลงนี้ช่วยเพิ่มทั้ง ความปลอดภัยและความสม่ำเสมอของโค้ด รวมถึงช่วยลดปัญหาที่ AI สร้างรายงานช่องโหว่ผิดพลาดได้

เบื้องหลังการถอด strcpy

  • cURL เคยลบการเรียก strncpy() ทั้งหมดมาก่อนแล้ว โดยชี้ปัญหาของฟังก์ชันนี้ว่าเป็น API ที่ไม่เป็นธรรมชาติ รับประกันการปิดท้ายด้วย null ไม่ได้ และมีการเติม 0 เกินจำเป็น
    • ในกรณีที่ต้องคัดลอกบางส่วนของสตริง ได้เปลี่ยนไปใช้ memcpy() และจัดการการปิดท้ายด้วย null เอง
  • แม้ strcpy() จะมี API ที่เรียบง่าย แต่เพราะ ไม่ได้ระบุขนาดบัฟเฟอร์อย่างชัดเจน จึงมีความเสี่ยงที่ระหว่างการบำรุงรักษา โค้ดตรวจสอบกับโค้ดคัดลอกจะถูกแยกจากกัน
    • หากโค้ดถูกแก้ไขต่อเนื่องเป็นเวลาหลายสิบปีโดยนักพัฒนาหลายคน ก็มี โอกาสที่การตรวจสอบขนาดบัฟเฟอร์จะถูกทำให้ไร้ผล

การนำฟังก์ชันคัดลอกสตริงแบบใหม่มาใช้

  • เพื่อป้องกันความเสี่ยงนี้ จึงมีการนำฟังก์ชันทดแทนชื่อ curlx_strcopy() มาใช้
    • รับอาร์กิวเมนต์เป็น บัฟเฟอร์ปลายทาง ขนาดบัฟเฟอร์ บัฟเฟอร์ต้นทาง และความยาวสตริงต้นทาง
    • จะทำงานด้วย memcpy() ก็ต่อเมื่อสามารถคัดลอกและปิดท้ายด้วย null ได้ครบถ้วนเท่านั้น
    • หากล้มเหลว จะตั้งค่าบัฟเฟอร์ปลายทางให้เป็นสตริงว่าง
  • ฟังก์ชันนี้ต้องใช้ อาร์กิวเมนต์และโค้ดมากกว่า strcpy() แต่ช่วยให้ การตรวจสอบบัฟเฟอร์ผูกอยู่ใกล้กับการคัดลอกโดยตรง จึงปลอดภัยกว่า
  • ในโค้ดเบสของ cURL จึงมีการสั่งห้ามการใช้ strcpy() อย่างสมบูรณ์ และถอดออกเช่นเดียวกับ strncpy()

รายละเอียดการติดตั้งใช้งาน

  • ตัวอย่างนิยามฟังก์ชันมีดังนี้ 
    void curlx_strcopy(char *dest, size_t dsize, const char *src, size_t slen)
{
  DEBUGASSERT(slen < dsize);
  if(slen < dsize) {
    memcpy(dest, src, slen);
    dest[slen] = 0;
  }
  else if(dsize)
    dest[0] = 0;
}
  • ใช้ DEBUGASSERT เพื่อ ตรวจจับข้อผิดพลาดได้ตั้งแต่ระหว่างพัฒนา และออกแบบให้ในสภาพแวดล้อมที่ใช้งานจริงทำงานสำเร็จเสมอ
  • ไม่มีค่าที่ส่งกลับเหมือน strcpy และเลือกใช้ แนวทางจับข้อผิดพลาดในขั้นตอนทดสอบและการทำ fuzzing

ปฏิกิริยาจากชุมชน

  • นักพัฒนาบางส่วนชี้ว่ามันคล้ายกับ strcpy_s() (C11 Annex K) แต่ cURL ยังคงใช้มาตรฐาน C89 อยู่
  • ความเห็นอื่น ๆ เสนอว่า ควรเพิ่มค่าที่ส่งกลับ หรือ ปรับปรุงวิธีจัดการเมื่อบัฟเฟอร์ล้มเหลว
  • ทาง cURL อธิบายว่า “ฟังก์ชันนี้ถูกออกแบบให้สำเร็จเสมอ จึงไม่จำเป็นต้องมีค่าที่ส่งกลับ”

ผลข้างเคียงที่เกี่ยวกับ AI

  • การเปลี่ยนแปลงครั้งนี้ช่วยป้องกัน ปัญหาที่แชตบอต AI ตรวจพบการใช้ strcpy ในโค้ดของ cURL ผิดพลาด แล้วอ้างว่า ‘มีช่องโหว่’ ได้
  • อย่างไรก็ดี ผู้เขียนก็ระบุว่า “AI ยังอาจสร้างรายงานเท็จแบบอื่นได้อยู่ดี” พร้อมกล่าวถึง ข้อจำกัดของการวิเคราะห์โค้ดด้วย AI

บทความที่เกี่ยวข้อง

5 ความคิดเห็น

 
ahwjdekf 2026-01-02

ควรใช้ snprintf แทน strcpy ถ้าในโค้ดยังมี strcpy อยู่ ก็ควรตามหาที่อยู่ของนักพัฒนาที่เขียนมันขึ้นมา
 
winmain 2026-01-02

นี่เป็นวิธีที่ผมเคยใช้ทำงานเป็นโค้ดดีบักตอนทำงานอยู่บริษัทเกมเมื่อ 25 ปีก่อน แต่จะมีแค่ strcpy ตัวเดียวเสียเมื่อไหร่กันล่ะ พอเป็นรีลีสก็ปลดกลับเพื่อเพิ่มความเร็วแล้วเอาไปให้บริการจริง ฝั่งเกมจริง ๆ ไวต่อปัญหาหน่วยความจำชนกันมากที่สุด เลยต้องทำงานกันอย่างระมัดระวังและมีสติสุด ๆ จนถึงขั้นทำเมมโมรีดีบักเกอร์ใช้กันเอง แต่พอมามองในวันนี้ ที่ทำอยู่นั่นมันเท่ากับกำลังสร้าง garbage collection อยู่เลย ช่างเป็นความทรงจำที่ชวนให้หวนคิดจริง ๆ
 
secwind 2026-01-02

ข้อผิดพลาด C4996 'strcpy' : ฟังก์ชันหรือตัวแปรนี้อาจไม่ปลอดภัย พิจารณาใช้ strcpy_s แทน หากต้องการปิดการเลิกใช้งาน ให้ใช้ _CRT_SECURE_NO_WARNINGS ดูความช่วยเหลือออนไลน์สำหรับรายละเอียด
 
GN⁺ 2026-01-01
ความคิดเห็นจาก Hacker News

  • strcpy() ไม่ดีแค่ในด้านความปลอดภัย แต่ยังแย่ในแง่ของ ประสิทธิภาพ ด้วย
    เมื่อก่อนเคยคิดว่าเวลาที่ไม่รู้ความยาวสตริง strcpy() จะมีประสิทธิภาพ แต่ในความเป็นจริงมันคัดลอกทีละไบต์ ทำให้ CPU ต้องคาดเดาการแตกแขนง และนี่ไม่มีประสิทธิภาพ

    • ตอนนี้คิดว่าควรเลิกใช้ null-terminated strings เองให้มากที่สุดเท่าที่จะทำได้
    • ช่วงหลังมานี้ไม่เคยเห็น strcpy ใช้ scalar loop เลย ไม่แน่ใจว่าเป็นแบบนั้นเฉพาะบนสถาปัตยกรรม ARM หรือเปล่า

  • รูทีนจัดการสตริงของ C แต่ละตัวต่างก็มี ข้อจำกัดใหญ่ จนรู้สึกว่าใช้งานจริงไม่ค่อยได้
    เลยคิดว่าจำเป็นต้องมีไลบรารีที่เก็บ ขนาดหน่วยความจำที่จัดสรรไว้ ควบคู่กับตัวชี้สตริง
    ตัวอย่างที่น่าดูคือ bstring library

    • strncpy ถูกสร้างขึ้นมาเพื่อคัดลอกชื่อไฟล์ที่มีความยาวคงที่ ดูรายละเอียดได้ใน คำตอบบน StackOverflow นี้
    • การที่สตริงไม่ได้เก็บข้อมูลความยาวไว้ เป็นผลจากการต้องการ ประหยัดหน่วยความจำ ในอดีต ตอนนั้นแม้แต่ไบต์เดียวก็มีค่า
    • เหตุที่ฟังก์ชันสตริงของ C ก่อปัญหา เป็นเพราะผู้ออกแบบเพิ่มมันเข้ามาโดย คาดการณ์ผลลัพธ์ตามมาได้ไม่เพียงพอ การที่อาร์เรย์ถูกบังคับให้แปลงเป็นพอยน์เตอร์เมื่อเป็นอาร์กิวเมนต์ของฟังก์ชันก็เป็นความผิดพลาดเชิงออกแบบพื้นฐานเช่นกัน
    • book-keeping เพิ่มเติมแบบนี้เมื่อก่อนถือว่าเป็นภาระ แต่ทุกวันนี้อยู่ในระดับที่รับได้สบายแล้ว
    • เดิมที strncpy เป็นฟังก์ชันสำหรับจัดการ ฟิลด์สตริงความกว้างคงที่ เช่นใช้เติม NUL ลงในฟิลด์อย่าง char username[20] ดูเอกสารที่เกี่ยวข้องได้ใน string_copying.7 manual

  • น่าแปลกที่ curlx_strcopy ไม่คืนค่าว่าทำสำเร็จหรือไม่
    แม้จะตรวจ dest[0] ได้ แต่แบบนั้น เสี่ยงต่อการทำให้เกิดข้อผิดพลาด และไม่ตรงไปตรงมา

    • เวอร์ชันก่อนหน้าคืนค่า error แต่ตอนนี้กลับล้มเหลวแบบเงียบ ๆ และตั้งเป็นสตริงว่างแทน ซึ่งดูแปลก
    • ดูเหมือนว่าจะถือว่าสำเร็จถ้า DEBUGASSERT(slen < dsize); ผ่าน แต่ใน release build นั้น assert อาจถูกตัดออก รหัสข้อผิดพลาดแบบชัดเจนน่าจะดีกว่า
    • ถ้าออกแบบแบบนี้ ก็มองว่าในอนาคตมีโอกาสเกิด CVE สูง

  • เดิมที strncpy() ไม่ได้มีไว้สำหรับ null-terminated strings แต่สำหรับฟิลด์ความยาวคงที่
    ปัญหาเริ่มจาก static analyzer แนะนำให้ใช้ strncpy แทน strcpy ทั้งที่ทางเลือกจริงคือ snprintf หรือ strlcpy

    • strlcpy เป็นฟังก์ชันสาย BSD จึงไม่อยู่ใน POSIX คำแนะนำอย่างเป็นทางการคือ stpecpy แต่แทบไม่มี implementation จริง ดู เอกสารที่เกี่ยวข้อง
    • เหตุที่ strncpy ต้อง padding หลัง NUL ก็เพื่อให้เปรียบเทียบ ฟิลด์ชื่อความยาวคงที่ อย่าง directory entry ได้อย่างมีประสิทธิภาพ ซึ่งในเอกสารประกอบมาตรฐาน ANSI C ก็ระบุไว้แบบนั้น

  • API นี้ให้ความรู้สึกเหมือน Annex-K มาก ขนาดของบัฟเฟอร์ปลายทางนับรวมพื้นที่สำหรับ NUL แต่ขนาดของต้นทางกลับไม่นับรวม
    คิดว่าเขียน memcpy ใช้เองตรง ๆ ยังดีกว่า

  • ในบทความมีประโยคที่น่าประทับใจว่า “strcpy คือ เหยื่อล่อให้ AI สร้างรายงานช่องโหว่ผิด ๆ”

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

  • หลักการที่ว่า “ตรวจสอบใกล้จุดใช้งานของโค้ด” เป็นสิ่งที่ดี แต่จะกำกวมเมื่อจำเป็นต้อง ตรวจสอบตั้งแต่ช่วงต้นของวงจรชีวิตข้อมูล
    น่าจะดีถ้ามีวิธีแยกข้อมูลที่ “ผ่านการตรวจสอบแล้ว” ด้วยชนิดข้อมูลแบบเดียวกับ Result ของ Rust

    • Result เก็บเพียงความสำเร็จ/ล้มเหลว ไม่ได้ รับประกันสถานะว่าผ่านการตรวจสอบแล้ว ดังนั้นควรมีชนิดข้อมูลแยกต่างหากที่สร้างได้ก็ต่อเมื่อผ่านกระบวนการตรวจสอบเท่านั้น นี่คือแนวคิด “** parse, don’t validate**”
    • ตามอุดมคติ การตรวจสอบไม่ควรทำใกล้โค้ดฝั่งผู้ใช้ข้อมูล แต่ควรทำ ให้เร็วที่สุดที่ขอบเขตของระบบ อย่างไรก็ตามสิ่งนี้ต้องอาศัย ระบบชนิดข้อมูลที่แสดงความหมายได้ชัดเจน
    • ในกรณีแบบนี้ การแยกใช้ชนิดข้อมูลเหมือน String และ CharSequence ของ Java ก็เป็นอีกวิธีหนึ่ง

  • ความต่างแบบ off-by-one ระหว่างขนาดบัฟเฟอร์กับความยาวสตริงเป็นปัญหาด้านการใช้งานที่เลวร้ายมาก และมีแนวโน้มจะทำให้เกิดข้อผิดพลาดต่อไปอีก

  • ฟังก์ชันคัดลอกสตริงที่เสนอขึ้นมาใหม่ หากคัดลอกไม่ได้จะ ล้างบัฟเฟอร์ปลายทางให้ว่าง แล้วคืนค่า void
    แต่คิดว่าในกรณีแบบนี้ควร จัดการเป็น error และไม่แตะต้องบัฟเฟอร์เลยจะดีกว่า การพึ่ง DEBUGASSERT อย่างเดียวไม่น่าไว้วางใจ

  • ขอแสดงความยินดีกับการทำโปรเจกต์นี้สำเร็จ C/C++ เองก็สามารถทำให้มี ความปลอดภัยของหน่วยความจำ ได้ถ้าพยายามมากพอ
    แต่ในสภาพแวดล้อมมือถือ ขนาดฟอนต์ของกราฟ เล็กเกินไปจนอ่านยาก

    • การเอา strcpy ออก ไม่ได้แปลว่าโค้ดจะ ปลอดภัยด้านหน่วยความจำ เสมอไป
    • ฟอนต์ของกราฟดูเหมือนออกแบบมาสำหรับงานพิมพ์ สำหรับบล็อกถือว่าเล็กเกินไป
 
hiongun 2026-01-04

ย้ายไปใช้ภาษา C3 เลยก็ดีนะครับ เป็นโปรเจ็กต์ที่คงไวยากรณ์ของภาษา C ไว้โดยเปลี่ยนแปลงให้น้อยที่สุด พร้อมเพิ่มฟีเจอร์สมัยใหม่เข้าไป ทำให้ย้ายไปใช้ได้ง่ายด้วยครับ