[error-report-web][PATCH V2] Add local.conf and auto.conf into error details


Richard Purdie
 

On Mon, 2021-03-22 at 15:32 +0000, Richard Purdie via lists.yoctoproject.org wrote:
Sorry about the delay on this, we do really need to get this resolved.
I'm wondering if we should replace the angled brackets test with
https://github.com/mozilla/bleach which would then remove the need
for these workarounds.

Would you be able to update the patch for the others issues please
and then we can look at this one separately?
I just sent out an as yet untested patch which may fix some of the quoting
issues by using bleach. I'd still need to add it to the requirements file...

Cheers,

Richard


Richard Purdie
 

On Fri, 2020-02-14 at 10:42 +0800, Changqing Li wrote:
On 12/11/19 1:45 PM, Changqing Li wrote:
On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT
changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

+        {% if detail.BUILD.LOCAL_CONF != "" %}
+        <dt></a>Local Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{
detail.BUILD.LOCAL_CONF | safe }}</dd>
+        {% endif %}
+
+        {% if detail.BUILD.AUTO_CONF != "" %}
+        <dt></a>Auto Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{
detail.BUILD.AUTO_CONF | safe }}</dd>
+        {% endif %}
We cannot use the safe filter here - doing so could open up an XSS
vulnerability, since anyone can upload anything to the error-report
application and the content could include links or other malicious
HTML data. We should allow it to be auto-escaped. Is there a
particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in
local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.
Sorry about the delay on this, we do really need to get this resolved.
I'm wondering if we should replace the angled brackets test with
https://github.com/mozilla/bleach which would then remove the need
for these workarounds.

Would you be able to update the patch for the others issues please
and then we can look at this one separately?

Thanks,

Richard


Armin Kuster
 

Changqing,

Please use bluelightning@... for now.

According to Linkedin, Paul is now at Microsoft.

- armin


On 2/13/20 6:42 PM, Changqing Li wrote:
Hi, Paul

Could you help to check my reply below, thanks.

On 12/11/19 1:45 PM, Changqing Li wrote:

On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@... wrote:
From: Changqing Li <changqing.li@...>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@...>
---
  Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
  Post/models.py                  |  2 ++
  Post/parser.py                  |  2 ++
  Post/test.py                    |  2 ++
  templates/error-details.html    | 10 ++++++++++
  test-data/test-payload.json     |  4 +++-
  6 files changed, 43 insertions(+), 1 deletion(-)
  create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
OK, thanks

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
      LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
      ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
                                    default=ErrorType.RECIPE)
+    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.
OK,  I will fix this.


+        {% if detail.BUILD.LOCAL_CONF != "" %}
+        <dt></a>Local Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+        {% endif %}
+
+        {% if detail.BUILD.AUTO_CONF != "" %}
+        <dt></a>Auto Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+        {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?

This is for resolve a problem when there is angle brackets in local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@...>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@...>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@...&gt;"

Do you have good idea to resolve this? Thanks.


Cheers
Paul




    


Changqing Li
 

Hi, Paul

Could you help to check my reply below, thanks.

On 12/11/19 1:45 PM, Changqing Li wrote:

On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
  Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
  Post/models.py                  |  2 ++
  Post/parser.py                  |  2 ++
  Post/test.py                    |  2 ++
  templates/error-details.html    | 10 ++++++++++
  test-data/test-payload.json     |  4 +++-
  6 files changed, 43 insertions(+), 1 deletion(-)
  create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
OK, thanks

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
      LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
      ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
                                    default=ErrorType.RECIPE)
+    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.
OK,  I will fix this.


+        {% if detail.BUILD.LOCAL_CONF != "" %}
+        <dt></a>Local Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+        {% endif %}
+
+        {% if detail.BUILD.AUTO_CONF != "" %}
+        <dt></a>Auto Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+        {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.


Cheers
Paul


Changqing Li
 

On 1/5/20 5:06 AM, Khem Raj wrote:
has this resolved on server side ? since I see that
patch to report-error.bbclass is still not applied in
oe-core
V2 patches is for resolve this problem,  and Paul have 3 comments.

so I need to send a V3 for fix the comments.  It is easy to fix first 2 comments,

but the last comments,  I don't have more proper way.

@Paul,  could you check my reply of comments #3?  Thanks.


On Mon, Dec 16, 2019 at 7:48 PM Changqing Li <changqing.li@windriver.com> wrote:
correct the mail list to yocto@lists.yoctoproject.org

On 12/11/19 1:45 PM, Changqing Li wrote:
On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT
changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error
report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6


This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
Post/models.py | 2 ++
Post/parser.py | 2 ++
Post/test.py | 2 ++
templates/error-details.html | 10 ++++++++++
test-data/test-payload.json | 4 +++-
6 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py
b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to
makemigrations) e.g. local_conf_auto_conf
OK, thanks
--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
LINK_BACK = models.TextField(max_length=300, blank=True,
null=True)
ERROR_TYPE = models.CharField(max_length=20,
choices=ERROR_TYPE_CHOICES,
default=ErrorType.RECIPE)
+ LOCAL_CONF =
models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+ AUTO_CONF =
models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the
MAX_UPLOAD_SIZE value after the fact would not change the database
structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the
overhead of the surrounding JSON would block it from being uploaded
if it did

However, since this is a TextField we don't actually have to specify
a max_length (for a TextField max_length only actually affects the
frontend, and we don't expose this field in a form) so it can just be
removed.

Another thing, instead of default="" you should use blank=True.
OK, I will fix this.

+ {% if detail.BUILD.LOCAL_CONF != "" %}
+ <dt></a>Local Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{
detail.BUILD.LOCAL_CONF | safe }}</dd>
+ {% endif %}
+
+ {% if detail.BUILD.AUTO_CONF != "" %}
+ <dt></a>Auto Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{
detail.BUILD.AUTO_CONF | safe }}</dd>
+ {% endif %}
We cannot use the safe filter here - doing so could open up an XSS
vulnerability, since anyone can upload anything to the error-report
application and the content could include links or other malicious
HTML data. We should allow it to be auto-escaped. Is there a
particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in
local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.

Cheers
Paul
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47707): https://lists.yoctoproject.org/g/yocto/message/47707
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/1997914
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub [raj.khem@gmail.com]
-=-=-=-=-=-=-=-=-=-=-=-


Khem Raj
 

has this resolved on server side ? since I see that
patch to report-error.bbclass is still not applied in
oe-core

On Mon, Dec 16, 2019 at 7:48 PM Changqing Li <changqing.li@windriver.com> wrote:

correct the mail list to yocto@lists.yoctoproject.org

On 12/11/19 1:45 PM, Changqing Li wrote:

On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT
changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error
report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6


This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
Post/models.py | 2 ++
Post/parser.py | 2 ++
Post/test.py | 2 ++
templates/error-details.html | 10 ++++++++++
test-data/test-payload.json | 4 +++-
6 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py
b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to
makemigrations) e.g. local_conf_auto_conf
OK, thanks

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
LINK_BACK = models.TextField(max_length=300, blank=True,
null=True)
ERROR_TYPE = models.CharField(max_length=20,
choices=ERROR_TYPE_CHOICES,
default=ErrorType.RECIPE)
+ LOCAL_CONF =
models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+ AUTO_CONF =
models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the
MAX_UPLOAD_SIZE value after the fact would not change the database
structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the
overhead of the surrounding JSON would block it from being uploaded
if it did

However, since this is a TextField we don't actually have to specify
a max_length (for a TextField max_length only actually affects the
frontend, and we don't expose this field in a form) so it can just be
removed.

Another thing, instead of default="" you should use blank=True.
OK, I will fix this.


+ {% if detail.BUILD.LOCAL_CONF != "" %}
+ <dt></a>Local Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{
detail.BUILD.LOCAL_CONF | safe }}</dd>
+ {% endif %}
+
+ {% if detail.BUILD.AUTO_CONF != "" %}
+ <dt></a>Auto Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{
detail.BUILD.AUTO_CONF | safe }}</dd>
+ {% endif %}
We cannot use the safe filter here - doing so could open up an XSS
vulnerability, since anyone can upload anything to the error-report
application and the content could include links or other malicious
HTML data. We should allow it to be auto-escaped. Is there a
particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in
local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass:
replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.


Cheers
Paul
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#47707): https://lists.yoctoproject.org/g/yocto/message/47707
Mute This Topic: https://lists.yoctoproject.org/mt/61340472/1997914
Group Owner: yocto+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/yocto/unsub [raj.khem@gmail.com]
-=-=-=-=-=-=-=-=-=-=-=-


Changqing Li
 

correct the mail list to yocto@lists.yoctoproject.org

On 12/11/19 1:45 PM, Changqing Li wrote:

On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
  Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
  Post/models.py                  |  2 ++
  Post/parser.py                  |  2 ++
  Post/test.py                    |  2 ++
  templates/error-details.html    | 10 ++++++++++
  test-data/test-payload.json     |  4 +++-
  6 files changed, 43 insertions(+), 1 deletion(-)
  create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
OK, thanks

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
      LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
      ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
                                    default=ErrorType.RECIPE)
+    LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+    AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.
OK,  I will fix this.


+        {% if detail.BUILD.LOCAL_CONF != "" %}
+        <dt></a>Local Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+        {% endif %}
+
+        {% if detail.BUILD.AUTO_CONF != "" %}
+        <dt></a>Auto Conf:</dt>
+        <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+        {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.


Cheers
Paul


Changqing Li
 

On 11/13/19 6:36 PM, Paul Eggleton wrote:
Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
Post/models.py | 2 ++
Post/parser.py | 2 ++
Post/test.py | 2 ++
templates/error-details.html | 10 ++++++++++
test-data/test-payload.json | 4 +++-
6 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf
OK, thanks

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
default=ErrorType.RECIPE)
+ LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+ AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.
OK,  I will fix this.


+ {% if detail.BUILD.LOCAL_CONF != "" %}
+ <dt></a>Local Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+ {% endif %}
+
+ {% if detail.BUILD.AUTO_CONF != "" %}
+ <dt></a>Auto Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+ {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?
This is for resolve a problem when there is angle brackets in local.conf/auto.conf.

I have a patch in oe-core [OE-core] [PATCH] report-error.bbclass: replace angle brackets with &lt; and &gt;]

when we have below content in local.conf or auto.conf:
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
send-error-report will fail with "HTTP Error 500: OK"

error-report-web do rudimentary check on all fields that are
passed to the graphs page to avoid any XSS happening, if contains
'<', the server will return error(Invalid characters in json).
fixed by use escape of <> to replace it.

NOTE: with this change, error-report-web need to add filter 'safe'
for the string wanted to display to avoid further HTML escaping
prior to output. Below is how the content displayed on webpage:
with the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj<raj.khem@gmail.com>"
without the filter 'safe':
BUILDHISTORY_COMMIT_AUTHOR ?= "Khem Raj &lt;raj.khem@gmail.com&gt;"

Do you have good idea to resolve this? Thanks.


Cheers
Paul


Paul Eggleton
 

Hi Changqing,

Some comments below.

On Tuesday, 12 November 2019 9:32:53 PM NZDT changqing.li@windriver.com wrote:
From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
Post/models.py | 2 ++
Post/parser.py | 2 ++
Post/test.py | 2 ++
templates/error-details.html | 10 ++++++++++
test-data/test-payload.json | 4 +++-
6 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
Could you please give the migration a proper name (-n option to makemigrations) e.g. local_conf_auto_conf

--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
default=ErrorType.RECIPE)
+ LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+ AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
I'm not sure this is practical, for two reasons:

1) Field sizes should not be variable like this; changing the MAX_UPLOAD_SIZE value after the fact would not change the database structure
2) The value could never actually reach MAX_UPLOAD_SIZE because the overhead of the surrounding JSON would block it from being uploaded if it did

However, since this is a TextField we don't actually have to specify a max_length (for a TextField max_length only actually affects the frontend, and we don't expose this field in a form) so it can just be removed.

Another thing, instead of default="" you should use blank=True.


+ {% if detail.BUILD.LOCAL_CONF != "" %}
+ <dt></a>Local Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+ {% endif %}
+
+ {% if detail.BUILD.AUTO_CONF != "" %}
+ <dt></a>Auto Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+ {% endif %}
We cannot use the safe filter here - doing so could open up an XSS vulnerability, since anyone can upload anything to the error-report application and the content could include links or other malicious HTML data. We should allow it to be auto-escaped. Is there a particular issue you were using this to solve?

Cheers
Paul

--

Paul Eggleton
Intel System Software Products


Changqing Li
 

From: Changqing Li <changqing.li@windriver.com>

Support to display local.conf and auto.conf on error report web.
Here is commit in oe-core, which add local.conf/auto.conf into error report
https://git.openembedded.org/openembedded-core/commit/?id=7adf9707c04d8ef6bcd8d8bda555687f705e6ee6

This commit is related to YOCTO #13252

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
Post/0006_auto_20190917_0419.py | 24 ++++++++++++++++++++++++
Post/models.py | 2 ++
Post/parser.py | 2 ++
Post/test.py | 2 ++
templates/error-details.html | 10 ++++++++++
test-data/test-payload.json | 4 +++-
6 files changed, 43 insertions(+), 1 deletion(-)
create mode 100644 Post/0006_auto_20190917_0419.py

diff --git a/Post/0006_auto_20190917_0419.py b/Post/0006_auto_20190917_0419.py
new file mode 100644
index 0000000..827944e
--- /dev/null
+++ b/Post/0006_auto_20190917_0419.py
@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('Post', '0005_build_error_type'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='build',
+ name='AUTO_CONF',
+ field=models.TextField(default=b'', max_length=5242880),
+ ),
+ migrations.AddField(
+ model_name='build',
+ name='LOCAL_CONF',
+ field=models.TextField(default=b'', max_length=5242880),
+ ),
+ ]
diff --git a/Post/models.py b/Post/models.py
index ddf2fc7..e9fd010 100644
--- a/Post/models.py
+++ b/Post/models.py
@@ -43,6 +43,8 @@ class Build(models.Model):
LINK_BACK = models.TextField(max_length=300, blank=True, null=True)
ERROR_TYPE = models.CharField(max_length=20, choices=ERROR_TYPE_CHOICES,
default=ErrorType.RECIPE)
+ LOCAL_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")
+ AUTO_CONF = models.TextField(max_length=int(settings.MAX_UPLOAD_SIZE), default="")

def save(self, *args, **kwargs):
if self.ERROR_TYPE not in [e_type[0] for e_type in
diff --git a/Post/parser.py b/Post/parser.py
index 9639308..69d43c1 100644
--- a/Post/parser.py
+++ b/Post/parser.py
@@ -53,6 +53,8 @@ class Parser:
b.EMAIL = str(jsondata['email'])
b.LINK_BACK = jsondata.get("link_back", None)
b.ERROR_TYPE = jsondata.get("error_type", ErrorType.RECIPE)
+ b.LOCAL_CONF = str(jsondata['local_conf'])
+ b.AUTO_CONF = str(jsondata['auto_conf'])

# Extract the branch and commit
g = re.match(r'(.*): (.*)', jsondata['branch_commit'])
diff --git a/Post/test.py b/Post/test.py
index 6dc7878..4fe236c 100755
--- a/Post/test.py
+++ b/Post/test.py
@@ -46,6 +46,8 @@ def compare_db_obj_with_payload(self, bf_object):
self.assertEqual(bf_object.BUILD.NAME == str(payload['username']), True)
self.assertEqual(bf_object.BUILD.EMAIL == str(payload['email']), True)
self.assertEqual(bf_object.BUILD.LINK_BACK == payload.get("link_back", None), True)
+ self.assertEqual(bf_object.BUILD.LOCAL_CONF == payload.get("local_conf", None), True)
+ self.assertEqual(bf_object.BUILD.AUTO_CONF == payload.get("auto_conf", None), True)

g = re.match(r'(.*): (.*)', payload['branch_commit'])

diff --git a/templates/error-details.html b/templates/error-details.html
index c30160d..c8fce05 100644
--- a/templates/error-details.html
+++ b/templates/error-details.html
@@ -81,6 +81,16 @@
{% endwith %}
</dd>

+ {% if detail.BUILD.LOCAL_CONF != "" %}
+ <dt></a>Local Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.LOCAL_CONF | safe }}</dd>
+ {% endif %}
+
+ {% if detail.BUILD.AUTO_CONF != "" %}
+ <dt></a>Auto Conf:</dt>
+ <dd style="white-space: pre-wrap;">{{ detail.BUILD.AUTO_CONF | safe }}</dd>
+ {% endif %}
+
</dl>
<div>
<a class="btn btn-block" target="_blank" href="https://bugzilla.yoctoproject.org/enter_bug.cgi?classification=__all" >Open a bug</a>
diff --git a/test-data/test-payload.json b/test-data/test-payload.json
index 0428a16..fd21053 100644
--- a/test-data/test-payload.json
+++ b/test-data/test-payload.json
@@ -1,4 +1,5 @@
{
+ "auto_conf": "",
"branch_commit": "(master-test): 736b49449233c936e3099b314a58e91ad17f9774",
"build_sys": "x86_64-linux",
"component": "base-passwd",
@@ -11,7 +12,8 @@
"package": "base-passwd-3.5.29-r0",
"task": "do_install"
}
- ],
+ ],
+ "local_conf": "",
"machine": "qemux86",
"nativelsb": "Ubuntu-14.04",
"target_sys": "i586-poky-linux"
--
2.7.4