wr_num 필드 값이 동시성 문제로 겹치는 경우 답변글에 대한 권한이 잘못 주어지는 등의 문제

[kkigomi]

글, 답변글을 작성할 때 get_next_num() 함수로 wr_num 필드의 값을 가져와 사용하는 과정에서 동시 또는 지연 등의 문제로 이 값이 같은 값이 사용될 수 있음.

gnuboard5/lib/common.lib.php

Lines 792 to 800 in 6416560

	// 게시판의 다음글 번호를 얻는다.
	function get_next_num($table)
	{
	// 가장 작은 번호를 얻어
	$sql = " select min(wr_num) as min_wr_num from $table ";
	$row = sql_fetch($sql);
	// 가장 작은 번호에 1을 빼서 넘겨줌
	return isset($row['min_wr_num']) ? (int)($row['min_wr_num'] - 1) : -1;
	}

문제 및 사례

이 원인으로 다음과 같은 문제가 발생할 수 있음

• 연관 없는 두 글이 wr_num으로 잘못된 그룹이 지어져 게시물 정렬에 문제가 발생할 수 있음
  • 사례 1: https://sir.kr/qa/466755
  • 사례 2: https://sir.kr/cm_free/944769
• 잘못된 연관 관계로인해 글 이동, 복사에 문제가 발생할 수 있음
  • 사례: https://sir.kr/qa/308125
    • 이 사례는 wr_num을 임의로 넣은 값이지만 중복이 발생할 때 같은 현상이 나타날 수 있음
• 답글이 있으면 글 삭제가 잘못 제한되는 문제가 있음
  • 사례: https://sir.kr/qa/452286

  • gnuboard5/bbs/delete.php

    Lines 44 to 52 in 6416560

    	// 원글만 구한다.
    	$sql = " select count(*) as cnt from $write_table
    	where wr_reply like '$reply%'
    	and wr_id <> '{$write['wr_id']}'
    	and wr_num = '{$write['wr_num']}'
    	and wr_is_comment = 0 ";
    	$row = sql_fetch($sql);
    	if ($row['cnt'] && !$is_admin)
    	alert('이 글과 관련된 답변글이 존재하므로 삭제 할 수 없습니다.\\n\\n우선 답변글부터 삭제하여 주십시오.');

• 회원이 작성한 비밀글에 답변글이 있을 때, 원글의 작성자에게 이 답변글을 볼 수 있는 권한을 부여할 때 엉뚱한 회원에게 권한이 부여될 수 있음
  • 비밀글로 설정된 내용에 대한 답변이 잘못된 권한 부여와 개인 정보등의 비밀 내용이 노출될 수 있음

  • gnuboard5/bbs/board.php

    Lines 84 to 96 in 6416560

    	// 회원이 비밀글을 올리고 관리자가 답변글을 올렸을 경우
    	// 회원이 관리자가 올린 답변글을 바로 볼 수 없던 오류를 수정
    	$is_owner = false;
    	if ($write['wr_reply'] && $member['mb_id'])
    	{
    	$sql = " select mb_id from {$write_table}
    	where wr_num = '{$write['wr_num']}'
    	and wr_reply = ''
    	and wr_is_comment = 0 ";
    	$row = sql_fetch($sql);
    	if ($row['mb_id'] === $member['mb_id'])
    	$is_owner = true;
    	}

• 이 외에도 wr_num에 의존하여 그룹을 만들어 글, 답글, 댓글에 대한 권한, 컨텐츠 변경 및 이동 등 연관지어 동작하는 코드에서 쉽게 발견되지 않는 알려지지 않는 문제가 발생할 수 있음

해결 방안

1. wr_num 값의 지연 문제 최소화

wr_num을 채워넣기위해 get_next_num()함수를 사용해 데이터를 가져오는 과정에서 예상보다 긴 지연이 발생할 경우 많은 문제를 일으킬 수 있으므로 이 함수를 사용하는 대신 서브쿼리 등으로 글 데이터를 insert 할 때 바로 참조하는 방법으로 조금이나마 개선이 가능할 것으로 보입니다.

이 문제의 완전한 해결책은 아니지만 현재의 호환성을 유지하면서 개선이 가능한 방법이 될 것 같습니다.

2. 답글의 wr_parent 필드 활용

대부분 '원글'을 잘못 찾는 문제이므로 댓글(comment)처럼 답글의 wr_parent에 원글의 wr_id를 넣고 원글을 찾는데 활용하는 것으로 해결이 가능할 것으로 보임.

다만, wr_parent가 댓글과의 연관 관계를 가지는데 사용된다면 답글의 댓글에 영향을 줄 수 있음. 정돈되지 않은 코드, 존재하지 않는 개발자 매뉴얼, 커밋 메시지와 코드 변경이력이 추적을 어렵게 하기 때문에 사실상 로직 분석이 불가하여 개발팀에서 해결 방안을 찾아야 할 것 같습니다.

[thisgun]

안녕하세요. SIR 입니다.

의견 주셔서 감사합니다.

[kkigomi]

@thisgun -$wr_id로 바뀌었군요.

이게 기준 값이 바뀌어버리는 셈인데 문제가 발생할 수 있습니다.

-$wr_id가 이 이슈의 중복 문제를 해결하는 가장 확실한 방법이지만, 그누보드 커뮤니티에서 흔히 "끌올(끌어 올리기)", "점프" 기능이라고 불리는 wr_num 컬럼의 값을 임의로 변경해 게시물을 가장 앞으로 정렬 시키는 팁이나 스킨, 플러그인 등으로 공유되는 것들이 있습니다. 저도 이런 기능을 만들다가 발견한 이슈입니다.

근데 이런 것들이 마찬가지로 get_next_num() 함수를 이용하는 방법으로 wr_num 컬럼의 값을 변경하는데 이번 변경사항을 아래와 같은 문제가 발생합니다.

3개의 글이 순서대로 작성한 상태 (정렬은 wr_num ASC)

wr_id	wr_num
3	-3
2	-2
1	-1

wr_id=1 글을 끌어올리기 (가장 작은 wr_num(-3)에서 1을 뺀 값(-4)으로 지정)

wr_id	wr_num
1	-4
3	-3
2	-2

새로운 글을 작성하면 wr_id=4로 글이 작성됨

wr_id	wr_num
4	-4
1	-4
3	-3
2	-2

이와 같이 wr_num이 겹치는 문제가 발생합니다.

이런 문제로 일명 "끌올" 기능을 활용하는 사이트에서는 wr_num 중복 문제가 더욱 심각해집니다.
"끌올" 기능을 빈번하게 사용할수록 기존 코드보다 우연이 아닌 확정적으로 문제가 발생합니다.

---

이 문제는 기존과 같이 get_next_num()을 사용했던 상황을 고려해 호환성을 유지하는 방법으로 서브쿼리로 대체하는 방법이 적절할 것 같습니다.

diff --git forkSrcPrefix/bbs/write_update.php forkDstPrefix/bbs/write_update.php
index d6d47647af35ec6f6a7f830fa5a711bb49b4c70d..6edd6fc69cffdb0bc1aee911448c476d1cf5d7a5 100644
--- forkSrcPrefix/bbs/write_update.php
+++ forkDstPrefix/bbs/write_update.php
@@ -259,7 +259,7 @@ if ($w == '' || $w == 'r') {
     }
 
     $sql = " insert into $write_table
-                set wr_num = '$wr_num',
+                set wr_num = (select min(wr_num) - 1 from $write_table sq),
                      wr_reply = '$wr_reply',
                      wr_comment = 0,
                      ca_name = '$ca_name',

물론 이번에 추가된 코드 $add_wr_update_sql = ($wr_num === 0) ? ", wr_num = '-$wr_id' " : ""; 등은 다시 제거되어야 합니다.

---

제가 이 이슈에서 wr_parent를 같이 언급한 이유가 기존 방식은 유지하되 wr_num을 서브쿼리로 값을 가져와 사용하여 지연 문제를 줄이도록 개선하고, 이는 완전히 문제를 해결하지 못할 수 있기 때문에 비밀답글의 권한이 잘못 주어지는 문제를 해결하기 위해서 답글의 wr_parent를 답글 자신의 wr_id가 아닌 원본 글의 wr_id를 기록하는 방식... 즉, "댓글(comment)"과 같은 방식의 적용이 가능한가에 대한 것입니다.

현재는 원본글과 마찬가지로 답글 또한 wr_id === wr_parent이므로 원본글을 찾는데 문제가 발생하지만 wr_parent 값을 댓글처럼 원본글의 wr_id를 사용한다면 원본글을 찾을 때 발생하는 문제를 해결할 수 있을 것 같아서 언급한 것입니다. 다만, 아직 관련 코드를 완전히 살피지 못해서 이 부분은 sir 개발자분들이 확인을 해주시길 바랐던 것입니다.

[kkigomi]

wr_parent 관련해서 코드를 다 살피지 못했기 때문에 PR이 아닌 이슈만 남겼던 것인데, 보안취약점은 어쩔 수 없겠지만 버그 등의 문제는 이슈 제출자가 변경된 코드를 확인하는데 시간을 좀 주셨으면 합니다.

이슈 내용을 나름대로 열심히 적었다고는 하지만 윗 댓글처럼 설명하지 못한 부분이나 부족한 부분이 있을 수 있습니다.
저도 업데이트하고 확인하다보니 아차 싶었습니다만, 이러면 제 이슈가 버그를 만들어 내는 원인이 되어버렸다는 생각에 철렁합니다.

내 이슈가 해결이 되었는지 확인하고 이슈 내용이 제대로 전달되었는지 이슈 제출자 스스로도 판단할 시간이 필요합니다.
이슈 제출자에게 3~7일 가량의 시간을 주시면 좋을 것 같습니다.

배포 전에 이슈나 코드에 대해 상호 점검할 시간이 주어졌으면 합니다.

[thisgun]

안녕하세요. SIR 입니다.

코드를 제공해 주셔서 정말 감사합니다.

알려주신 코드를 참고하여 코드를 수정했습니다.