Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

템플릿 보안 향상을 위한 auto-escape 적용 범위 확대 #2301

Closed
ghost opened this issue Sep 18, 2018 · 30 comments
Closed

템플릿 보안 향상을 위한 auto-escape 적용 범위 확대 #2301

ghost opened this issue Sep 18, 2018 · 30 comments

Comments

@ghost
Copy link

ghost commented Sep 18, 2018

#2300 이슈에 더해 추가 개선을 적용합니다.

  • 제외되었던 레이아웃 편집기 등에서 PHP 코드 사용 제한
    • PHP 코드가 포함되면 요청이 거부되며, 변경사항을 잃을 수 있습니다
    • 위지윅 에디터를 통한 글쓰기에는 제한되지 않습니다
  • URL 요청 시 parameter에 script 등의 태그 사용 제한
    • 일반적으로 영향이 없으나, 특정 플러그인에 따라 링크 또는 주소창에 보여지는 항목에 HTML 태그가 사용되면 제한됩니다
  • XE 1 배포본에 포함된 모듈, 레이아웃, 스킨에서 화면에 출력하는 사용자 입력 값 중에 HTML 태그가 포함되면 태그가 적용되지 않고 일반 문자열로 표시됩니다
    • 배포본에 포함된 항목에만 적용되며, 배포본에 포함된 파일을 수정하여 특정 값을 출력하는 경우 영향을 받을 수도 있습니다
    • 예) 메뉴 설명에 HTML을 입력하고, 배포본에 포함된 XEDITION 레이아웃에 메뉴 설명(HTML)을 출력하는 경우
    • 이 사항은 배포본에 포함된 파일을 대상으로하지만, 추후 업데이트에서 플러그인 제작자 또는 사이트 운영자가 선택적으로 이러한 보안 옵션을 사용할 수 있는 기능이 추가될 수 있습니다

템플릿 필터 사용법

@kijin님의 코드로부터 기인한 템플릿 필터 기능이 추가되었으며, 이 기능이 템플릿 보안 향상의 역할을 합니다.

출력 값에 대한 escape 필터를 적용할 수 있으며, 사용 방법은 아래와 같습니다.
escape 필터는 htmlspecialchars() 함수를 사용하는 간소화된 방법입니다.

escape 필터의 종류

  • escape : 출력되는 결과물에 htmlspecialchars()가 적용되며, double_encode 옵션은 true입니다
    • 일반적으로 사용되지 않을 수 있으며, textarea 필드에 출력될 값이 대상이 될 수 있습니다
  • autoescape : 출력되는 결과물에 htmlspecialchars()가 적용되며, double_encode 옵션은 false입니다
    • HTML이 적용되지 않아야 할 항목에 사용
  • noescape : 별도 처리하지 않고 그대로 출력합니다
    • HTML을 적용해야 할 항목에 사용

각 필터는 아래 <config>를 이용한 일괄 설정에 대한 필터를 개별 항목에 덮어 쓸 수 있습니다.

escape 적용

HTML 템플릿 파일 상단에 아래와 같이 명시하여, 해당 파일에서 출력하는 모든 값에 autoescape 필터를 적용할 수 있습니다.

<config autoescape="on" />

템플릿 전체에 적용하지 않고 일부 출력 값에 적용은 아래와 같습니다.

<div class="content">
  {$content|autoescape}
</div>

이 두 방법을 혼용하여, 필터를 덮어 쓰면 아래와 같이 동작합니다.

<!-- 해당 템플릿 파일 전체에 autoescape 필터 적용 -->
<config autoescape="on" />

<div class="content">
  <!-- autoescape 필터 적용 -->
  <!--
  	$content = '<strong>내용</strong>'
  		-> '&lt;strong&gt;내용&lt;/strong&gt;'
  	$content = '홈 &gt; 게시판'
  		-> '홈 &gt; 게시판' // 변경 없음
  -->
  {$content|autoescape}

  <!-- double_encode=true인 escape 필터 적용 -->
  <!--
  	$content = '&lt;strong&gt;내용&lt;/strong&gt;'
  		-> '&amp;lt;strong&amp;gt;내용&amp;lt;/strong&amp;gt;' // double_encode
  	$content = '홈 &gt; 게시판'
  		-> '홈 &amp;gt; 게시판' // double_encode
  -->
  {$content|escape}

  <!-- escape 필터를 적용하지 않음 -->
  <!--
  	$content = '<strong>내용</strong>'
  		-> '<strong>내용</strong>' // 변경 없음
  	$content = '홈 &gt; 게시판'
  		-> '홈 &gt; 게시판' // 변경 없음
  -->
  {$content|noescape} 
</div>

아래와 같이 공백이 포함되지 않도록 주의해야 합니다(아래와 같은 사항은 무시되며, 오류가 발생할 수 있습니다).

  • {$content | noescape}
  • {$content| noescape}
  • {$content |noescape}

즉, pipe 문자(|) 앞 뒤에는 공백이 포함되지 않아야 합니다.


이 변경 사항은 최초 안내한 것처럼 '모든 템플릿 파일'을 대상으로 했던 것과 다르게, XE 1 배포본에 포함돈 파일을 대상으로 합니다.

@ghost ghost added the type/enhancement label Sep 18, 2018
@ghost ghost self-assigned this Sep 18, 2018
ghost pushed a commit that referenced this issue Sep 18, 2018
ghost pushed a commit that referenced this issue Sep 18, 2018
- 내장 모듈의 template 파일에 대한 autoescape 기본 활성화
  - `modules/**/tpl/**/*.html`
  - `common/tpl/*.html`
- 설치 시 위 대상을 포함한 전체 템플릿에 대해 autoescape 기본 활성화
  - files/db.config.php 파일에서 'safeguard' 값을 'N'을 변경하여 비활성화 가능
- getUrl() 등 알려진 함수 및 주요 변수들에 대해 escape 지정 해제
  - 해당 템플릿 코드에서 명시적으로 재지정 가능
@ghost
Copy link
Author

ghost commented Sep 18, 2018

  • 내장 모듈의 template 파일에 대한 autoescape 기본 활성화
    • modules/**/tpl/**/*.html
    • common/tpl/*.html
  • 설치 시 위 대상을 포함한 전체 템플릿에 대해 autoescape 기본 활성화
    • files/db.config.php 파일에서 'safeguard' 값을 'N'을 변경하여 비활성화 가능
  • getUrl() 등 알려진 함수 및 주요 변수들에 대해 escape 지정 해제
    • 해당 템플릿 코드에서 명시적으로 재지정 가능

ghost pushed a commit that referenced this issue Sep 19, 2018
- 템플릿에 명시적으로 지정한 config 설정에 따라 escape 옵션 반영
- 결과물 또한 조정되어 test 수정
@ghost ghost added this to the 1.11.0 milestone Sep 19, 2018
@kijin
Copy link
Contributor

kijin commented Sep 19, 2018

허허... 일이 커지네요 ㄷㄷㄷ

예전에 Object를 BaseObject로 변경할 때 삼항식을 사용해서 구버전, 신버전 모두 호환되도록 하는 방법을 알려주었듯이, 이것도 1.9 이하 버전과 1.11 이상 버전에서 모두 호환되는 서드파티 자료를 만드는 방법을 미리 공지하셔야 할 것 같습니다.

예를 들어 escape하지 않아야 하는 변수가 있다면 상단에 <config autoescape="off" /> 태그를 넣거나 {$var|noescpae}로 표현해야 하는데, 이런 템플릿을 구버전에서 사용하면 전자는 엉뚱한 태그가 출력될 수 있고, 후자는 | 문자를 bitwise OR로 해석하여 엉망이 될 수도 있거든요.

@ghost ghost changed the title 템플릿 escape 추가 개선 템플릿 보안 향상의 위한 auto-escape 적용 범위 확대 Sep 19, 2018
ghost pushed a commit that referenced this issue Sep 19, 2018
@ghost
Copy link
Author

ghost commented Sep 19, 2018

@kijin 개발자나 사이트 운영자가 가질 선택의 문제가 되겠네요.
1.11 버전을 기준으로 선택하거나, 분기하거나, safeguard 옵션을 끄도록 안내하거나.

말씀하신 문제에 대해서는 관련 공지 게시물에 추가 해두겠습니다.

@bjrambo
Copy link
Contributor

bjrambo commented Sep 19, 2018

@bnu XE코어가 현재까지 오게 될 수 있었던 것은 그래도 기존의 XE 1.4~1.8시절동안 만들어진 모듈들과 레이아웃 디자인 등등이 있어 왔기 때문에 현재까지 유지가 가능했어요.

적어도 호환성에 문제가 된다면 1.11버전기준으로 각 버전이 분리되면서 1.11 버전에 이 이슈에 적힌 내용들이 적용이 된다면 사용자 운영진 입장에서는 1.11을 선택할 메리트는 없어질 것 같습니다.

현재 만들어진 자료중에 유지보수가 더 이상 되지 않는 모듈들이 90퍼센트 이상 방치 되어있다는 가정하에 있다면 정작 PHP7.2이슈를 해결할 BaseObject이슈를 해결하지 못한 서드파티들도 XE자료실에서 반이상일텐데.. 이러한 자료들을 하나하나 찾아가면서 XE팀에서 호환성을 유지하기 위해 고쳐주지 않는다면 기존의 이용자 입장이나 신규 이용자 입장이나 더 이상 패치를 올릴 의미가 사라질 수 있다는 것입니다.

기존의 서드파티중 보안의 취약점에 걸리지가 않는다면 호환성이 완벽하게 보장된다는 가정하에 기능을 추가해야하는 방향이 맞는 것 같아요. :)

오픈소스방향으로 나아가는 XE인만큼 사용자의 편의성도 조금 생각해주시면 감사하겠습니다 (_ _ )

ghost pushed a commit that referenced this issue Sep 19, 2018
ghost pushed a commit that referenced this issue Sep 19, 2018
@ghost
Copy link
Author

ghost commented Sep 19, 2018

@bjrambo safeguard 값을 'N'으로 변경할 수 있는 선택 사항이 있습니다.
이 값을 변경하기 쉽도록 설정 페이지 내에 제공할 예정입니다.

ps. 이러한 변경은 사용자나 개발자를 힘들게 하기 위함도 아니며, XE3로 사용자를 유도하고자 위함도 아니며, 추측할 수 있는 사이트 운영자 또는 개발자를 포함하는 저희 외의 그 어떤 사람에게 영향을 끼칠 어떠한 불합리적인 목적도 가지고 있지 않습니다.

보안 향상을 위한 목적만을 가지며, 비록 호환성 문제를 발생 시킬 수 있으나 사용자가 선택할 수 있는 옵션을 제공하고 있고, 업데이트 사용자는 이 변경 사항을 즉시 적용받지않고 선택에 따라 켤 수 있도록 했습니다.
safeguard off 상태에서도 코어에서도 문제는 계속 발견될 수 있으나, 계속해서 잡아가고 있습니다.

저희의 결정이 모두 올바르지 않을 수 있으며, 죄송하게도 사이트 운영에 막대한 영향을 줄 수 있는 결정이 포함될 수도 있습니다. 다만, 이러한 결정은 더 나은 것을 위한 목적을 가지며, 저희는 그 어떤 이도 괴롭히고 싶지 않으며, 등 떠밀어 떠나가라 말하는 것도 아닙니다.

비록, 업데이트 시 안전장치는 허술하나 사용자는 선택할 수 있습니다.

@bjrambo
Copy link
Contributor

bjrambo commented Sep 19, 2018

그 동안 XE사용자가 패치를 하게 되면서 어쩔수이 겪게된 호환성이라던지 여러가지 고려를 생각했을때 기존자료들과의 호환성이 가장큰 문제점으로 다가오는 이슈이다보니 저도 걱정이 되어서 댓글을 작성하게 되었습니다.

옵션을 사용하더라도 기존자료와 99%까지 호환성을 이끌어낼 수 있다면 더할 나위 없는 좋은 패치인건 인정합니다.

응원합니다 👍

@conory
Copy link
Contributor

conory commented Sep 19, 2018

서드파티의 제작날자를 체크하여 신규 서드파티에 한해서만 우선적으로 적용시키면 어떻까요?

@ghost
Copy link
Author

ghost commented Sep 19, 2018

@bjrambo 호환성을 확보할 수 있는 부분은 최대한 찾아내고 싶고, 적정선에 대해서도 고민하고 있습니다.

@conory 좋은 아이디어 같습니다. 다만, 날짜로 구분하는 것은 다소 모호하다고 생각됩니다. 모듈이나, 위젯 등이 가지고 있는 xml 파일에 어떤 설정 값을 두는 것도 생각해보았으나, 사용자는 그보다 간단한 safeguard를 끄는 방법이 있고 개발자는 https://www.xpressengine.com/devlog/23277227 이 글에 명시한 '방법1'을 적용한다면 xml 파일에 명시하는 것 또한 무의미 할 수 있습니다.

추가 아이디어가 있으면 알려주세요 :)

@conory
Copy link
Contributor

conory commented Sep 19, 2018

safeguard를 끄면 신규/기존 할 것 없이 모든 autoescape가 끄질 겁니다. 물론 신규 서드파티에서 템플릿마다 <config autoconfig="on"> 선언하면 되긴하겠지만 귀찮고, 또 이것을 모르는 제작자들은 기존 방법대로 계속 만들 겁니다. 최신버전에서 안되면 방법을 찾겠죠.

기존 서드파티의 호환성도 확보하고, 또 앞으로의 보안성도 확보하고, 이것이 궁극적인 목적이라고 생각됩니다. 그래서 특정시점에 따라 '신규/기존'로 구분하는 것도 필요하지 않을까 해서 말씀드려봤습니다.

@ghost
Copy link
Author

ghost commented Sep 19, 2018

@conory 문제는 상위 호환이 안 되기 때문에 1.9 사용자는 변경된 템플릿 규격을 사용한 레이아웃을 어떤 방법을 동원해도 사용하지는 못 합니다.

그래서 '방법1'과 같은 처리로 호환성을 확보하는 방법 외에는 할 수 있는게 없기는 합니다. 크게 어렵지 않은 방법이기도 하고요.

@kijin
Copy link
Contributor

kijin commented Sep 19, 2018

@bnu @conory safeguard 설정에 따라 어떤 경로의 템플릿은 autoescape를 적용하고, 어떤 경로의 템플릿은 autoescape를 적용하지 않는 로직이 이미 있을 것 같습니다. 즉 템플릿 파일의 경로에 따라 판단하는 것이지요. 그렇다면 서드파티 자료 적용 여부도 날짜나 버전이 아니라 경로에 따라 판단하도록 한다면 어떨까요?

예를 들어

Context::addAutoEscapeTplPath('./modules/board/skins/myskin');

이렇게 하면 ./modules/board/skins/myskin 아래의 모든 템플릿에는 autoescape를 적용하는 것이지요. 보안에 신경 좀 쓰는 신규 자료는 이것을 한 번씩 실행해 주도록 하고 (또는 xml 파일의 설정값에 따라 자동으로 등록해 주고), 코어 모듈에 포함된 템플릿 등 autoescape 호환성이 확보된 것은 미리 이 리스트에 추가해 놓고요.

물론 이것은 opt-in 방식이기 때문에 99%의 사용자들은 전혀 켜지 않고 사용할 가능성이 높습니다 ㅜㅜ

방법 1은 너무 지저분합니다.

@ghost
Copy link
Author

ghost commented Sep 19, 2018

1.11의 템플릿 필터는 1.9 이하의 사용자에겐 문제를 일으키며, 업데이트하지 않은 1.9 사이트... 상위 호환성을 갖는 것은 불가능합니다. 하지만 레이아웃, 스킨 등 템플릿을 사용하는 자료의 개발자가 하위 호환성 확보를 위한 목적하에 1.11과 1.9이하에서 모두 가능한 방법으로 @ echo를 사용하는 것이고, 하위 호환성 확보를 가능케 합니다. 이는 지저분하지 않으며, 좀 더 이뻐야 할 필요도 없으며, 아주 이뻐야 할 이유도 없습니다. 게다가 템플릿을 다루는 개발자가 보안을 해제하지 않고 필요한 부분에서만 처리하여 호환성을 확보할 수 있는 간단한 해결 방법입니다.

자료의 날짜로 구분을 하거나, xml 파일에 특정 flag를 두거나 PHP 코드로 '나를 안전하게 다뤄줘'라고 선언을 하거나, 특정 경로로 판단하는 등의 복잡한 방법을 찾지 않고 그러한 추가적인 방법을 따르도록 하지 않고도:

  • 안전을 확보하기 위해 safeguard를 끄지 않고: 'Y'로 설정하고
  • 템플릿을 통째로 무장 해제하지 않기 위해: <config autoescape="off">로 설정하지 않고
  • 필요한 부분에서만: {$content|noescape} 필터를 적용하고
  • 추가로 하위 호환성을 확보해야한다면: {@ echo $content}

이와 같이 설정하는 것이 안전 확보와 하위 호환성 확보를 위한 방법으로 생각합니다. 물론 @ echo보다 좀 더 이쁜 방법이 있다면 그걸 사용해도 될 것입니다.

사이트 운영자 선택을 위해 safeguard 옵션을 두었지만 이를 해제하지 않을 것을 권장하고, 하지만 운영상 문제가 있다면 이 옵션을 꺼서 호환성을 확보할 수 있습니다.
템플릿을 직접 다룰 때는 복잡한 설정 없이 필요한 곳에서만 noescape 필터를 사용하거나 @ echo를 덧붙이면 보안을 해제하지 않고 호환성을 확보할 수 있습니다.
템플릿을 사용하는 자료를 배포하는 개발자는 1.11 이상만 지원할 것이라면 템플릿 필터를 사용할 수 있고, 1.9이하도 지원해야한다면 noescape 필터 대신 @ echo를 사용하여 하위 호환성을 확보할 수 있습니다.

@kijin
Copy link
Contributor

kijin commented Sep 19, 2018

@bnu 모범답안을 말씀하셨지만, 실제로 XE가 사용되는 현장에는 적용하기 어려운 답안입니다. 개발자들이 흔히 생각하는 이상이라고 할까요? 이상은 여기에서 찾으시고 XE1에서는 현실적인 판단을 내려 주시기 바랍니다. 보안도 이상이 아닌 현실이기 때문입니다.

대다수 XE 사용자들이 사용하는 서드파티 자료들은 벌써 오래 전에 지원이 중단된 것이 현실입니다. 그런 자료를 사용하기가 불편해진다면 대다수의 일반 사용자들은 시간과 돈을 들여 고치기보다는 그냥 XE 1.9에 머무를 가능성이 평소보다 더 높아진다는 것도 현실이고, 그렇다면 이후에 나오는 보안패치의 혜택도 받지 못하게 된다는 것이 현실입니다. 언젠가 또 발견될지도 모르는 보안취약점을 차단한다는 명분으로, 많은 사용자들을 잘 알려진 구체적인 보안취약점에 노출시키는 결과를 낳는 것이지요.

XE 기반으로 신규 오픈하는 사이트가 많지 않은 상황이라 대부분의 사이트는 앞으로도 safeguard = N 상태로 운영될 가능성이 높은데, 그렇다면 앞으로 safeguard = N 상태에서만 발생하는 보안취약점에는 어떻게 대응하실 것인지도 궁금합니다. safeguard = Y 로 변경하는 것을 권장하니까 굳이 패치하지 않으실 건가요? 게다가 safeguard 설정, 즉 "XE를 처음 설치한 날짜"에 따라 보안과 직결된 기능의 작동 방식이 달라진다면 클린 인스톨을 분석하는 연구자들은 오히려 중대한 (현실적으로 많은 사용자들에게 피해를 줄 수 있는) 보안취약점을 발견하지 못하고 넘어갈 가능성이 높아질지도 모른다는 생각이 드는군요.

코어와 함께 배포되는 템플릿에 autoescape를 적용하는 것은 적극 찬성합니다. 그러나 서드파티 자료에 기본 적용하는 것은 반대합니다. 적용하는 옵션을 제공한다 해도 "XE를 처음 설치한 날짜" 따위에 좌우되지 않고 항상 동일한 기본값(N)을 가져야 한다고 생각합니다.

제가 관리하는 10여개의 서드파티 자료에서는 어떤 형태로든 autoescape 대응을 하지 않고, 만약 문제점이 발견될 경우 safeguard = N 으로 변경할 것을 권하도록 하겠습니다.

@ghost
Copy link
Author

ghost commented Sep 19, 2018

@kijin safeguard = N 상태에서도 발생하는 문제에 대해서는 제가 이 저장소에 커밋/푸시 권한이 있는한 개별적으로 추가 대응합니다. 만약 팀에서 그러한 필요성이 없다고 입을 모은다면 퇴근 후 저녁밥 먹고 패치하면 됩니다.

관리하시는 자료에 대한 결정은 개발자의 선택이므로 첨언하지 않습니다.

ps.
말씀하신 대로 이상은 다른데서 찾고 있으며, 여기에서 이상을 찾을 시간은 사실 없습니다.
이상을 찾는 것이 아니며, 현실의 문제를 타파하기 위함입니다.

@ghost ghost changed the title 템플릿 보안 향상의 위한 auto-escape 적용 범위 확대 템플릿 보안 향상을 위한 auto-escape 적용 범위 확대 Sep 19, 2018
ghost pushed a commit that referenced this issue Sep 19, 2018
ghost pushed a commit that referenced this issue Sep 20, 2018
@ghost ghost added the type/SECURITY label Sep 21, 2018
@ghost
Copy link
Author

ghost commented Sep 27, 2018

ignoreEscape 목록에 있는 것 중 getUrl 등 명확하고 사용이 빈번한 것은 남겨두고, $content, $skin_content 등 일부에서 사용되고 빈도가 낮은 패턴은 제거하도록 하겠습니다. 아래 목록 정도를 남겨둘 것 같네요(정확한 패턴이 아닌 예시입니다).

Context::getHtmlHeader()
Context::getHtmlFooter()
Context::getBodyHeader()
getUrl()
getNotEncodedUrl()
getAutoEncodedUrl()
getFullUrl()
getNotEncodedFullUrl()
getSiteUrl()
getNotEncodedSiteUrl()
getFullSiteUrl()
getCurrentPageUrl()
getCurrentPageUrl()
htmlspecialchars()
nl2br()
$lang->

@kijin
Copy link
Contributor

kijin commented Sep 27, 2018

@bnu 변경하신 정책 및 autoescape 적용여부 표기 방식에 동의합니다^^

서드파티 자료에 기본 적용하지 않는다면 예외 문법 목록도 최대한 단순화하거나 아예 없애는 것이 좋겠습니다. 말씀하신 목록 중 헤더/푸터 불러오는 함수는 common_layout.html과 mobile_layout.html에서만 사용할 테니 거기서 noescape 필터를 적용하면 그만이고, URL 처리 관련 함수는 autoescape를 적용하더라도 double escape = false 상태가 기본값이니 상관없지 않나요? getNotEncoded*Url()은 모듈에서 redirect할 때나 필요한 것이지, 템플릿에서 사용할 때는 이것도 autoescape해도 될 것 같고요.

autoescape 설정의 적용 범위가 문제네요. 모듈/애드온/위젯의 xml 파일에서 선언하면 해당 모듈/애드온/위젯의 모든 스킨과 tpl 폴더 내의 템플릿에 적용되고, 스킨이나 레이아웃의 xml 파일에서 선언하면 해당 스킨이나 레이아웃에만 적용되는 것인가요? 이 경우 모듈에서는 autoescape를 선언했는데 스킨에서는 false로 지정할 수 있나요? 애당초 스킨이나 레이아웃에서만 사용할 수 있는 옵션이라고 하면 간단할 것 같기는 합니다만...

자료에 포함된 xml 파일이 제 기능을 한다면 autoescape 지원 여부를 굳이 웹상에 표시할 필요는 없을 것 같습니다. 모두 자동으로 처리되는 거라 최종 사용자에게는 구분이 필요하지 않으니까요. 몇 년째 거의 변화가 없는 자료실에 오랜만에 새로운 항목을 추가한다면 차라리 PHP 버전 지원 범위, XE 코어 버전 지원 범위가 우선이라고 생각됩니다. 릴리즈 추가할 때마다 XE 코어를 의존성으로 선언하는 것도 무척 귀찮거든요;;;

@ghost
Copy link
Author

ghost commented Sep 27, 2018

@kijin 어렵네요.
xml 파일에 표기하는 것은 다음 버전/추가 이슈로 넘기겠습니다.

getHtmlHeader 얘네들은 제거하고, getUrl 등은 parameter가 붙으면 & 문자열이 문제입니다.

@kijin
Copy link
Contributor

kijin commented Sep 27, 2018

@bnu getUrl류의 함수들 대부분은 이미 &&amp;로 변환한 상태로 반환하는데, 거기에 autoescape를 추가 적용(double escape = false)하더라도 &amp;amp;가 되지는 않을 텐데요... NotEncoded 버전을 템플릿에서 사용하는 사례만 좀 찾아보면 되겠어요.

@ghost
Copy link
Author

ghost commented Sep 27, 2018

@kijin 그러게요. 제가 코드를 다듬던 중에 문제가 발생해서 추가했었는데, autoescape가 아니고 escape가 적용된 상태에서 확인된 현상이었는지도 모르겠습니다. 대부분을 제거해도 되겠네요.

ghost pushed a commit that referenced this issue Sep 27, 2018
ghost pushed a commit that referenced this issue Sep 27, 2018
@howtoxe
Copy link
Contributor

howtoxe commented Sep 27, 2018

@bnu 아.. 제 경우는 딱히 예외목록을 문제삼은 것보다도 왜 $content가 예외목록에 있는데도 배포본, 예제에서 다시 이스케이프를 하셨는지 궁금했던 것이었어요. 혼동을 줄 수 있을 것 같아서요.

그리고 htmlentitiesstrip_tags도 추가하는게 좋지 않을까요?

@kijin
Copy link
Contributor

kijin commented Sep 27, 2018

@howtoxe 예외를 더 추가하는 것은 반대입니다. 어차피 서드파티 자료에는 적용하지 않겠다고 @bnu 님이 결정하셨으니, 코어만 제대로 고쳐놓으면 예외는 전혀 없어도 됩니다.

htmlentities는 코어에 포함된 스킨에서 사용하는 예가 없고, 사용해서도 안 됩니다. (사실 htmlspecialchars도 잘못 사용하는 예가 너무 많아서, 모두 다시 인코딩하는 것이 안전합니다.)

strip_tags 함수는 위험한 코드를 모두 제거한다는 보장이 없습니다. 태그를 모두 지웠더니 사이사이에 교묘하게 넣어두었던 내용들이 맞닿아서 오히려 더 위험한 코드가 나오기도 합니다.

@ghost
Copy link
Author

ghost commented Sep 27, 2018

<sc<p>ript>alert()</</p>script> 이런거 위험하죠.

@howtoxe
Copy link
Contributor

howtoxe commented Sep 27, 2018

@kijin 이해했습니다. 설명 감사합니다.

근데 strip_tags는 필터목록에도 이스케이프 예외로 포함이 되어있더라구요. 이 부분도 오용 가능성이 있지 있을까요?

@ghost
Copy link
Author

ghost commented Sep 27, 2018

@howtoxe 제가 일괄 추가했던 부분인데. 제거해야겠네요.
지적 감사합니다.

@ghost
Copy link
Author

ghost commented Sep 27, 2018

근데 이제보니 제거해주긴 하네요. str_replace 같은걸로 잘못 제거할 때는 위험하지만 strip_tags에서는 잘 제거해주는데, 다른 패턴이 있었는지 기억나질 않네요.

@ghost
Copy link
Author

ghost commented Sep 27, 2018

?url=" onm<>ouseover="ale<>rt(0) 요런게 있네요.

ghost pushed a commit that referenced this issue Sep 27, 2018
ghost pushed a commit that referenced this issue Oct 8, 2018
ghost pushed a commit that referenced this issue Oct 8, 2018
ghost pushed a commit that referenced this issue Oct 8, 2018
ghost pushed a commit that referenced this issue Oct 8, 2018
ghost pushed a commit that referenced this issue Oct 8, 2018
@ghost ghost closed this as completed Oct 10, 2018
ghost pushed a commit that referenced this issue Oct 10, 2018
ghost pushed a commit that referenced this issue Oct 10, 2018
dorami added a commit to daolcms/daolcms that referenced this issue Nov 7, 2018
dorami added a commit to daolcms/daolcms that referenced this issue Nov 7, 2018
dorami added a commit to daolcms/daolcms that referenced this issue Nov 9, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants