From a7d775654569555f8a848deb5b6248cad38f2b77 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Wed, 2 Jan 2019 12:39:00 +0100
Subject: [PATCH] Fix #374: Strip EXIF metadata from uploaded avatars to avoid
 leaking private data

---
 api/funkwhale_api/common/serializers.py |  26 ++++++++++++++++++++++++
 api/funkwhale_api/users/serializers.py  |  10 +++++++--
 api/tests/common/exif.jpg               | Bin 0 -> 4278 bytes
 api/tests/common/test_serializers.py    |  19 +++++++++++++++++
 changes/changelog.d/374.enhancement     |   1 +
 5 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 api/tests/common/exif.jpg
 create mode 100644 changes/changelog.d/374.enhancement

diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py
index e253906d..e7bbf8f1 100644
--- a/api/funkwhale_api/common/serializers.py
+++ b/api/funkwhale_api/common/serializers.py
@@ -1,8 +1,12 @@
 import collections
+import io
+import PIL
+import os
 
 from rest_framework import serializers
 
 from django.core.exceptions import ObjectDoesNotExist
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.utils.encoding import smart_text
 from django.utils.translation import ugettext_lazy as _
 
@@ -190,3 +194,25 @@ def track_fields_for_update(*fields):
         return serializer_class
 
     return decorator
+
+
+class StripExifImageField(serializers.ImageField):
+    def to_internal_value(self, data):
+        file_obj = super().to_internal_value(data)
+
+        image = PIL.Image.open(file_obj)
+        data = list(image.getdata())
+        image_without_exif = PIL.Image.new(image.mode, image.size)
+        image_without_exif.putdata(data)
+
+        with io.BytesIO() as output:
+            image_without_exif.save(
+                output,
+                format=PIL.Image.EXTENSION[os.path.splitext(file_obj.name)[-1]],
+                quality=100,
+            )
+            content = output.getvalue()
+
+        return SimpleUploadedFile(
+            file_obj.name, content, content_type=file_obj.content_type
+        )
diff --git a/api/funkwhale_api/users/serializers.py b/api/funkwhale_api/users/serializers.py
index bcacc314..c75604f6 100644
--- a/api/funkwhale_api/users/serializers.py
+++ b/api/funkwhale_api/users/serializers.py
@@ -11,7 +11,7 @@ from rest_framework import serializers
 from versatileimagefield.serializers import VersatileImageFieldSerializer
 
 from funkwhale_api.activity import serializers as activity_serializers
-
+from funkwhale_api.common import serializers as common_serializers
 from . import models
 
 
@@ -66,7 +66,13 @@ class UserActivitySerializer(activity_serializers.ModelSerializer):
         return "Person"
 
 
-avatar_field = VersatileImageFieldSerializer(allow_null=True, sizes="square")
+class AvatarField(
+    common_serializers.StripExifImageField, VersatileImageFieldSerializer
+):
+    pass
+
+
+avatar_field = AvatarField(allow_null=True, sizes="square")
 
 
 class UserBasicSerializer(serializers.ModelSerializer):
diff --git a/api/tests/common/exif.jpg b/api/tests/common/exif.jpg
new file mode 100644
index 0000000000000000000000000000000000000000..c13141aff10c549e994468090649702c5c13c086
GIT binary patch
literal 4278
zcmd5-XFyZgwmv5b9f1TwQ4j=_I+%nOx`Ko%NR=i^QJRG&O;Cym(g`BP29YixDAGn|
zK!KqM2t<k?B2`2>gJqoI?a<`i_kO&0f8CwzbH20A+H0?G@3p_PKCnItap;ruNDvIe
zkS_Q^>mQgpwFBJFLD2EzkR${_j1UXV4j}+y0-X$VY+w$6@4^@e3GOiXLGWIPZUaM5
zF}!aFSHR+1J|F^&-x$jR{GP*v4a@~_3xe~fPk8!<ofd02$_+wo#T<hm<a3gRz8*=>
z@HhmK+%BFy;TE8vgu~ew8v_GG%bk|T=3Phc_z59`jGT--1QBpJqB2fVSr#vZS5yXn
zc?C!aG~ml0es4Qd1Tk9pXBN?7Ti6+bk|2ly_@ZzGJT3{joi*Y}5^4vVC&QaQFailB
zBX(lCl3|D$j0JfiJjp-p0c7NkJ(P^v!O>*8ov}$|hOHdHT9BExFbGCw-tkE%2XD+f
z58`KKZRd%{;j-W@e=u_vVrLbdvyeO3Ad7ArBkxn#wqqg_DC|4<Hics=4%lZ3&lUzd
z`EYYCn6~{7H@SlX+HNpdGRPUMJ6;(_P{u0>;S`kRaLPD&+Ftw*8}JnZ9jp`5lr|15
z*T#6jX>RiwT)|6Vm)?4Q3+~L@)xZiK(4aGH(81wlaS#Urlr02}fDUsYH_C#RDPju)
z2qAZ2jx9_pax@3?hW`}6Lc4qp?83NR7{3b>w(!PEpf~5G%>`k0Va}fz-h~mnFl`7)
z&q#Z-NH>-SSl9x<1%%$ZBLJo)j51Xs0&1J}hXZ>LY~a*_t)WqdA$oB6U(YE@_~$uA
zN$x(U2za|PPz1zI_0ZkgeNH!K`Il3-zFSXdbAxQxhafG8jft6+nTd^+g_WJ1jRPyp
zjpgLTO70ip6DCN@$>F7OhZWV0wG@>NR1V{`?MViv=GNBM@|q5=XDnTePg+~hg23$T
z>{tv|oSR$RLJ6m2@&9h?^$?m7B0>g8SQtW}VI&$}Z-fLvo^(L{Xacy^z#&G_(K9eI
zF|z=NXF%{^Bocu_($S%S9Kj*9LykhzVGiJr(sP<PFbI2b5yFzQ8AUY98?dI`i=wiQ
z-r-El+&sK|dk-EG6PJ*blUGnADk*Cn(<bTY>gk)ATUeg7vbJ$@K6}o^)y>_<_p+aV
zKwwbB^~f7h(KlnrDYsH@r=@3PQXb~y=H(X@7Cm`d@vO3{x~BHU%f_bWKU!MbdU~mE
z`uYb3-+mYypO~DQ{y6h#X?bP!&$TaKzkR3i0t&E6Ylqpt@InJ#2ows5VxaMY5&obf
z(I~nDczVoH69xw_PGLeABbR1!c6kGnh^*-%*3rA0nOjuu!@(sQwGC$fH)7%c7PB2<
zKY6`{SdpOY(MU9;0oAAaV8W<DnHIm3iNVoBEbj$AO-EO>tj4ZYCX9E&N4viG4D=ll
zA!3`e^K$zO9@NIO_!9}_0DcSe)V@oyzxiaAjkkxL@C>3p80bILJ{L)`T8ZP?R~K|5
z!9%h9nMc9Fj>;J}(Kwe`Ywvf%c3qR2)>Zs)zr0;_Rp_z&sf$+Ykp7#+x(Vfw>5N~V
z8ZGaMP0*jJ`Nox-og)&$<ok|*Rhg$?mbz=Oeq&S3uXAo^k(T%iESlU$Y(j2u%<;?{
zEAh{D_*BKBI*(!K7ZS$E*DKQ}xt}R}@bp}Ez1l0S)gdBXQ#^RFq%jTy6|q5G`Y)eU
zD?Cv!f~FT2tkZ1%x^t6NSHUH&Z|TA#zrEzjT$PN8+Kk-fSlPSeE4?MzhqSvM%MFD2
zF~^cUyoX2`a$o9N4zMu_Jhd^>6A@^`NRUm2*>u+mWJ*Pc2ix+z8BTW=8~JIr5uN&H
ztgrJOY}Ugv_bJ?nnTW?98@akPV_DH*WMF$e)vLycMDid^R%r<Egc<4x^~OA@xwEV$
zqk;XlRIqG-jDIPdonqJH;MkQ1k*=OO(&00*KRnR+R1+mSoHf#?Sxt(xVEmVdP)!P&
zge&B24(Y06^CA@$O;Q{BE~BUyGtOtZQp6(pQ%;md1-G%M#H}5wZI{~TVaMpmvOuN2
z8gy!MoVC?`$RI*~_BVA1Zy)c#+>}Lkmf;mLV%{UHm%?+hfG3Ha_*m=LfRfQCH+0NR
z#~1iRx{V&y<Q5M$e{Jh<qzX`kq(TG!T(DcksVg+cy$XO>`<;pgS$=K4?Wdw5zM6AW
zxaw3{bAei<W??p)jn(vp=d!Ae@-G=r_7;}J9^e0>oz8KE{~G7YN!|zc6@5hFtR6K*
zcs%3lc$nV6m0qiGPxV`+U$33zDn|AhUR0|!rRSZ>a%prLre08BldxhfG;pmgnc(2S
zOT2yOSy$9CU*c03a?j;LJBDn8?~9Rg^Ovt1lB?*@zH}itY8^VoG5e{argfDP(=a22
zR-Y&@o%(JjO7z5ca9L0o^}hWvZRn<MsWhw21-0`3c-TV`6O%L1WnUlsPt#um$QcPA
z_^H49z_*JgRNLYUK4^5nLJu<l5hHAAFC79!Py3ny_1(~3AiZ=zdxcFLyuw6)Kye)`
zZ|Ghm$eMyNe4E1eGv4MHI`HZMkUUxg60+T3pd|+|c*0Q>r?BSwf0LJE=DdnxVrxa#
zZ5Qt9`Xmn#J3J2tDKD$z&-ZzJD;1QP-e3P&%GU37$pMwMym8%?P|;fzxrs`p5tPfS
z;x|b3qk`^(B3?WLXUDJY+2`gce{r8l*G!Y>oq`S#EW@Rd>aj|jl%uCK@@`IO`4kt5
zVx_e;B63$-#@}F{>;B=CQZT(>+}b8n2N!+(-ROB#-mqH5JS*m2)ET$huh;UFzvY|Q
z`(KoNzqce$)xN+2U!Gv83ybGN)FS1w?w9RTFGg&eO>Q)z!^K?=4vkfa*h($C^a#t0
zGAlbxHgXPFrn&HP3g*w+4$=o&n5A^pQ{G7-EcveaIt$|(BY9gFjDiJUrxfsAEM=#t
zC5H^XpR*hI(Aw==d={G=r)&_~(8;J8!&!KwttoNO@D*yg9l>mkNk>z!JHq+iA9boT
zAvR8!=4E%wz5buw|I}>gMb!P4Ff!Sr$Ay27mFPKZmwcN4vhLxa(3p2ThC+{Y=|5j<
z@@{IKJH<jV^LrtkVxD!M{?Cff)@87gyx4?Tb_v-x;QXX*d*H!ph~w>|ygyPP>aUnn
z(@ng7VRC^{4tGaQor7|0!}VYBPzS}_eY&})#=eV}<hw>It*!~IEoh?OMDc%Z%NQB<
z{g_t?TQMD3hcfsdWKJDUD{M%&ry5odJf^=NKxwlGJ?^G5bkm0Cv{!<}`7{H^IJ>*|
zsKGscAE({ZZ;S=X9CCBMWX7MF8bo(TrQ&+IYX77C{wlr90_QV@{pS4&%0Jy|;Ze6A
zpEa2`#9O{r>rS0K=P)DYY|mwhFZcg`<u&<DT8e~pV{4>(vbME-D_2Zgla)a95Z98O
zN#|criz6DVF!e<UKB%E<>KjGp=6uPRofg`myo0OoUU~5Nnf?)Cs)MtKl>x4Ps=7TR
z{}azIB8X+B58Cc_HA(GyJ<N$Ozm$eWAL}}&LM{AzN#snCIcuh5XP-wE3nnF{b-*H5
z(h{1D4gAH6wa9|{JuG|PC>rrt^pgg*i|2C=cax7wT60MvapKLL#Vq1L<%|3F$$Djm
zqkYLOM~es|>JK~{`t^nd9_8Ox`SqxzT7|I}A^c%QAb-`@T<;up39m{iWA?zrsZ+z;
zPBkpYi$9mlm6bI=n!nPoV3=TYXt4OYhl+V2vzOnJ6tTa)^zlo}_Fsn2>`4onZCfQS
zl&<P7)P|jDRMDzmhhC3=KM4mLYNu8@dg_!fm_^KC6LOH4v=E&+(`mo@;XOr<yfKE&
zQf{S!$MCXMWn7=yU-V-*`i;sDmkoE{rE2eg>pNtC-EViM>_<lph2^&Die1S-NfQ6>
zJ&*UX%AUI&YJRCspUWvJ*|&|cbf^J0p_&uSUU%8~<D=8r!ztJB)mOVoJPZG1jw&z4
zjdWzOn>qjOE-@eX{v%s|-te$XTU^*+!i!9s06)fhk9>k+*?`$fj&ypDXbg%8<sPFJ
zspDkgT`pay!uq99aO#q~1hhib9GuF&fKSY9Q0))Jn?^!YPL0b&60Kvu#eQ%_DY)Zn
zE`5}ugyi@dw=~I^y3YToLFe8!i|ia_Q^?7C!J8v9Te8xAQbSTqjFh{mFh2I2K}0Eg
zP`05)18ZvJso234NAmi9vZj}NIMatug(K$Kp^*A4PO-E-rlF`g#*csdE%U)r$fSs2
zZw>b=y}=f$Xx-xE8LQ5_tZs)MtABoeuf@zQV6mo_fx+_qNUhu18}oGc5sVIGwRkO2
zzx2nAjL8FU(&A?^auz)qYO^BoSN2Pg@v$ip4}#W6L_~Rv%0EI_J<(UG8FA^8@%r=(
zOMzXdtHsEqt1kuLh9>?~^_}_Y4A(#Ld2e~^2QC@@ko#;o$+Q~DASf>pP(ENiRgvf^
q=_Q2G8I560w6!pFUWd|V&Al(jur?k2VUzuCmbYNRG%Rs_@V@}5h1c!?

literal 0
HcmV?d00001

diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py
index bf04e8d2..6d20443a 100644
--- a/api/tests/common/test_serializers.py
+++ b/api/tests/common/test_serializers.py
@@ -1,3 +1,8 @@
+import os
+import PIL
+
+from django.core.files.uploadedfile import SimpleUploadedFile
+
 import django_filters
 
 from funkwhale_api.common import serializers
@@ -163,3 +168,17 @@ def test_track_fields_for_update(mocker):
         {"field1": "value1", "field2": "value2"},
         {"field1": "newvalue1", "field2": "newvalue2"},
     )
+
+
+def test_strip_exif_field():
+    source_path = os.path.join(os.path.dirname(__file__), "exif.jpg")
+    source = PIL.Image.open(source_path)
+
+    assert bool(source._getexif())
+
+    with open(source_path, "rb") as f:
+        uploaded = SimpleUploadedFile("source.jpg", f.read(), content_type="image/jpeg")
+    field = serializers.StripExifImageField()
+
+    cleaned = PIL.Image.open(field.to_internal_value(uploaded))
+    assert cleaned._getexif() is None
diff --git a/changes/changelog.d/374.enhancement b/changes/changelog.d/374.enhancement
new file mode 100644
index 00000000..71d3e911
--- /dev/null
+++ b/changes/changelog.d/374.enhancement
@@ -0,0 +1 @@
+Strip EXIF metadata from uploaded avatars to avoid leaking private data (#374)
-- 
GitLab