ทดลอง Shellshock บน OS X

พอดีเห็นว่าบน Mac มันทดลองง่ายดี ยังไม่มี patch ออกมาด้วย

Screen Shot 2557-09-27 at 11.35.31 PM

 

OS X มี web server ติดมาในตัว ใช้รัน CGI ง่ายๆได้ แค่เอาไฟล์ไปวางไว้ใน /Library/WebServer/CGI-Executables/ ยกตัวอย่างไฟล์​ test.sh

 

Screen Shot 2557-09-27 at 11.44.32 PM

ไม่เคยเขียน CGI web app ด้วย bash มาก่อน (อันนี้ก็อันแรก ด้วยความอยากรู้)​ ตอนรันก็แค่เปิด http://localhost/cgi-bin/test.sh

Screen Shot 2557-09-27 at 11.46.37 PM

เป็นไปตามที่คิด — เข้าเรื่อง Shellshock เราก็จะฝังพวก command เข้าไปใน HTTP headers บางตัว ซึ่งโดยการทำงานของ CGI แล้ว headers พวกนี้ก็จะถูกเอาไปทำเป็น environment variables สำหรับตัว bash ที่จะถูกเรียกขึ้นมา (อ่านเรื่อง environement var และ CGI ที่นี่) ถ้าเปลี่ยนโค้ดนิดหน่อยเป็น

echo -e “<html><body><pre>$(env)</pre></body></html>”;

ก็จะเห็นชัดขึ้นว่ามันเข้าไปอยู่เป็น environment variables จริงๆ

Screen Shot 2557-09-27 at 11.55.16 PM

ลองใช้คำสั่ง curl แล้วฝังคอมมานด์ชั่วร้าย (!) ลง header เพื่อใช้ Shellshock ดู

curl -H ‘Referer:() { :; }; echo “uh oh! muhahahahha” > /tmp/msg_from_me.txt‘ http://localhost/cgi-bin/test.sh

แน่นอนว่า content ดังกล่าวก็จะไปโผล่ในไฟล์ /tmp/msg_from_me.txt

คำถาม: วิธี attack แบบนี้ทำได้เฉพาะกับ CGI ที่เขียนด้วย bash เหรอ สมัยนี้ใครเค้าจะทำอะไรแบบนี้กัน! ไปเขียน Java (!) สิครัช

คำตอบ: ถ้ามีวิธียัด command ใดๆใส่ environment variables แล้วเรียก bash (รุ่นที่มีปัญหา) ขึ้นมาทำงานได้ มันก็โดนเล่นงานได้หมด

บังเอิญว่ากลไกของ CGI มันเอาข้อมูลบางส่วนใน HTTP request ไปใส่ใน env ให้เองเสมอ ก็เลยเป็นช่องทางให้เล่นงานได้

ยกตัวอย่างเช่น Perl

Screen Shot 2557-09-28 at 12.18.45 AM

(ต้องเรียก bash ตรงๆใน backticks ไม่งั้นมันใช้ shell ตัว default ตัวอื่นนะจ๊ะ)

หรือถ้าเป็น PHP ที่รันบน Apache ที่ใช้ mod_cgi และเรียก bash ขึ้นมาทำงานผ่าน exec() หรืออะไรก็ตามแต่ ก็มีความเสี่ยงเช่นกัน (ไม่สันทัด PHP เลยไม่ได้ลองเล่นดู)

เข้าใจว่าสมัยนี้ไม่น่าจะมีคนรัน CGI web app แล้วนะ ตัวเลือกดีๆอย่างอื่นมันเยอะกว่า แต่ก็ยังอยากเห็นช่องทางโจมตีช่องอื่นอีก สำหรับคนดูแลระบบแค่ update bash ไปน่าจะง่ายสุด

ขอบคุณ Everything you need to know about the Shellshock Bash bug ที่ทำให้เห็นว่ามันทดลองง่ายดีจริงๆ

Share Button

โค้ดรีวิว

งานที่ผมทำก่อนหน้านี้ไม่ใช่ software engineer หรือ programmer แต่มันมีส่วนที่ได้เขียนโค้ดบ้าง ซึ่งมักเป็นโปรเจค “เครื่องเคียง” ซะเป็นส่วนใหญ่ กล่าวคือ เอ็งทำเสร็จก็ดี (หรือดีมาก) แต่ไม่เสร็จก็ไม่ได้กระทบอะไรกับทีมนักหนา

ดังนั้น การเขียนโค้ดของผมช่วงทำงาน 3.5 ปีแรกมันไม่ได้มีโปรเซสอะไรมาก ค่อนข้างลูกทุ่ง (ภาษาอังกฤษใช้คำว่า “cowboy”) คือ เขียนเอาผลลัพธ์อย่างเดียว เสร็จใช้งานได้ดีตาม requirements เป็นโอเค – Get shit done!

ส่วนหนึ่งที่ไม่เคยทำคือ โค้ดรีวิว (code review)

ดังนั้นการเปลี่ยนแปลงที่สำคัญที่ผมเจอหลังมาเริ่มงานกับ “อากู๋ให้ 5 ดาว” (ให้ห้าดาวนั่นไม่เกี่ยว แต่ถ้าคุณเก็ต แปลว่าคุณแก่มากกกกก) คือการได้สัมผัสการเขียนโค้ดแบบที่ต้องให้คนอื่นรีวิวและเห็นด้วยก่อนจึงจะ submit ได้ ดังนั้นการรีวิวจึงเกิดขึ้นอยู่ตลอดเวลา ไม่ได้ทำวันละครั้งหรืออาทิตย์ละครั้ง

เชื่อหรือไม่ว่า ว่ากลุ่มคนผู้รักการเขียนโค้ดส่วนใหญ่ (รวมตัวเอง)​ มักจะคิดว่าโค้ดของตัวเองดีเลิศประเสริฐศรี สวยงาม ปลอดบั๊ก แต่ก็นั่นแหละ เชื่อหรือไม่(อีก)ว่า การให้ตาที่สอง หรือ สาม สี่ ห้า มาดูด้วย เราจะเห็นอะไรที่เรามองไม่เห็นและเป็นประสบการณ์เรียนรู้ที่ดีเลยทีเดียว ส่วนคนรีวิวก็มีโอกาสได้อ่านโค้ดเราและเรียนรู้อะไรที่ไม่เคยรู้เช่นกัน

ขโมยมุกคนอื่นมา
ขโมยมุกคนอื่นมา

ข้างล่างเป็นตัวอย่างเท่าที่จำได้ พอหอมปากหอมคอ ให้เห็นภาพว่าในโค้ดรีวิวเค้าคุยเรื่องอะไรกันบ้าง (แน่นอนว่ามันไม่ใช่โค้ดที่เขียนจริงๆ)​

ตย. 1

public HashMap<Integer, String> getIdToNameMapping(String path) {

ตย. 2

if node.index >= mid and not node.visited:
  return True
else
  return False

ตย. 3

# Count the number of brown dogs and print a list of names.
brown_dogs = [d for d in dogs if d.color == 'brown']
print "There are %s dogs: %s % (len(brown_dogs), ', '.join(brown_dogs))

ตย. 4

class A {
public:
 A { x_ = new B(); }
 ~A { delete x_; }
private:
 B* x_
};

ตย.5

logger.warning(String.format(
  "Could not parse data for item: %s", itemId));

ตย.6

class CoffeeMaker {
  private CoffeeBean bean;
  public CoffeeMaker(CoffeeBean bean) {
    this.bean = bean;
  }
  // ...
}

ตย.7

bool CreateTableFromFile(string path, Table& table);

ผลลัพธ์ที่ได้จากการรีวิว

ตย. 1
ฟังก์ชันมัน return HashMap ซึ่งจริงๆแล้วคนเรียกฟังก์ชัน​(อาจจะ)ไม่ได้สนใจว่ามันจะเป็น Map ประเภทไหน (ขอให้เป็น Map ก็พอ) ดังนั้นเราควรจะ return type ที่มัน general ที่สุด ในที่นี้ก็เป็น Map แทน

public Map<Integer, String> getIdToNameMapping(String path) {

ตย. 2
unnecessary if/else อันนี้เป็นอะไรที่รู้อยู่แก่ใจแต่หลายๆครั้งเขียนโค้ดแก้ๆลบๆ เลยหลุดไปบ้าง ต้องให้คนมาเห็นแล้วเตือน

return node.index >= mid and not node.visited

ตย. 3
ข้อนี้น่าจะเป็นอะไรที่น่าแปลกใจสำหรับเราชาวไทยผู้ภาษาอังกฤษไม่แข็งแรง (ใช่ ไม่ได้เกี่ยวกับโค้ดเลย..) คือมันมีประโยคภาษาอังกฤษที่เรียกว่า ประโยคคำสั่ง (หรือ imperative) และประโยคบอกเล่า (descriptive) ซึ่งชาวต่างชาติเจ้าของภาษาเค้าคงมองสองอันนี้แยกออกจากกันได้ชัดเจน ทีนี้เนี่ย ประโยคที่ใช้เขียนคอมเม้นต์ใน code มันควรจะเป็นประโยคบอกเล่า ดังนั้นโค้ดข้างล่างแค่เติม s ลงไปที่ verb ก็พอ (ให้ตัว code เป็นประธาน)

# Counts the number of brown dogs and prints a list of names.

ตย. 4
มีข้อตกลงกันภายใน (code standard, style หรือ convention อะไรก็แล้วแต่)​ ว่าให้หลีกเลี่ยง raw pointer ดังนั้นตรงนี้มันควรจะเป็น std::unique_ptr แทน เหตุผลประกอบดูได้ใน C++ style guide

class A {
public:
 A { x_.reset(new B()); }
private:
 std::unique_ptr<B> x_
};

ตย.5
บางอย่างก็ขึ้นกับรสนิยมส่วนตัวของ reviewer แต่ละคน อย่างคนนี้มักจะชอบคอมเม้นต์เรื่องการใช้ String.format อย่างฟุ่มเฟือย ในเคสที่มันใช้ + ก็น่าจะเพียงพอ

logger.warning("Could not parse data for item:" + itemId));

ตย.6
เป็นเรื่องของ Java ที่ผมไม่เคยคิดว่าคนอื่นเค้าให้ความสำคัญกัน คือบาง field ที่เป็น final ได้ควรจะเป็น final (อารมณ์เดียวกับเรื่อง const nazi ของ C++)

class CoffeeMaker {
  private final CoffeeBean bean;
  public CoffeeMaker(CoffeeBean bean) {
    this.bean = bean;
  }
  // ...
}

ตย.7
เหมือนจะเป็นเรื่องที่ชาว C++ เค้าเข้าใจตรงกัน ว่าสำหรับ argument ที่เป็น input มันก็ควรจะเป็น const หรือ const reference (จะได้มั่นใจว่าฟังก์ชันที่เรียกไม่ได้แก้ไข input) และ output ก็ควรจะเป็น pointer (เพื่อเป็นสัญญาณว่าชั้นได้ pointer มา ชั้นจะทำอะไรก็ได้) กลายเป็น

bool CreateTableFromFile(const string& path, Table* table);

ตัวอย่างพวกนี้เป็นน้ำจิ้มมากๆ จริงๆแล้วมีปัญหาระดับที่ใหญ่ขึ้นมาอีก เช่น

  • ออกแบบ classes มาไม่ดี ต้องกลับไปโละใหม่หมด – อันนี้แก้ได้โดยการไปคุยออฟไลน์กับคนรีวิวก่อน ว่าวางแผนจะทำแบบนี้ๆ มี issue อะไรมั้ย
  • แพตช์ที่ส่งไป ใหญ่เกินไป ทำหลายอย่างมากเกินไป – ทำให้รีวิวยาก ใช้เวลานาน ผมคิดว่าประมาณ 500 บรรทัดนี่เริ่มตึงๆแล้ว ดังนั้นพอเสร็จเป็นจุดๆไปก็ควรส่งเข้าไปรีวิวทันที อย่าดองไว้นาน
  • เขียนโค้ดแล้วไม่มีเทสต์มาคู่กัน

ทำโค้ดรีวิวก็ใช่ว่าจะไม่มีข้อเสีย ถ้าเจอ reviewer สนใจเรื่องหยุมหยิมมากๆ (ตั้งชื่อตัวแปรไม่ถูกใจ, indent code ที่มันยาวไปหลายบรรทัดไม่เหมือนกัน, วิธีเรียง member ใน class, … !@#$@#$) ก็ทำให้เสียเวลาโดยไม่ได้อะไรกลับมามาก หรือถ้า reviewer ไม่มีเวลา หรืออยู่คนละประเทศ ก็ทำให้เราทำงานช้าลงได้

หลายๆครั้ง เราอาจไม่ต้องการโค้ดรีวิวเลยก็ได้ ยกตัวอย่างเช่น ถ้าคุณทำเว็บ marketing ให้นมตราหมี (ทำไมต้องนมตราหมี)​ ซึ่งแน่ใจว่าใช้งาน 4 เดือนหมด campaign แล้วโยนทิ้งแน่นอน เราจะสนใจ code quality มากเท่าทำงานให้เสร็จทันเวลาหรือไม่? เป็นเรื่องที่ต้องคิด ส่วนบริษัทหลายแห่งที่บังคับให้โค้ดรีวิวเป็นส่วนหนึ่งในการทำงาน คงคิดมาระดับหนึ่งแล้วว่า เวลาที่เสียไปกับการรีวิวคงคุ้มค่ากับการที่ engineer ไม่ต้องมาเสียเวลาปวดหัวทำความเข้าใจโค้ดเน่าๆ

โดยส่วนตัวผมได้เรียนรู้อะไรจากการที่มีคนรีวิวโค้ดให้เยอะมาก โดยเฉพาะคนที่เขียนโค้ดแล้วตั้งใจให้อยู่คู่โลกไปนานๆ (เท่จัง​)​ การทำโค้ดรีวิวคงเป็นสิ่งที่ขาดไม่ได้เลยทีเดียว

(รูปหัวบล็อกเอามาจาก Chromium หยิบมาแบบมั่วๆ)

Share Button