diff --git a/Changelog.md b/Changelog.md index a755c8eecb06f7e8b4b2eb3d4f7142bcbc6fc25f..5167e1d76240193ffe7fc07d22009ed3b0d4dd05 100644 --- a/Changelog.md +++ b/Changelog.md @@ -60,6 +60,12 @@ bind to an UNIX socket at `unix:tmp/diaspora.sock`. Please change your local * Add configuration options for some debug logs [#6090](https://github.com/diaspora/diaspora/pull/6090) * Send new users a welcome message from the podmin [#6128](https://github.com/diaspora/diaspora/pull/6128) +# 0.5.1.2 + +diaspora\* versions prior 0.5.1.2 leaked potentially private profile data (namely the bio, birthday, gender and location fields) to +unauthorized users. While the frontend properly hid them, the backend missed a check to not include them in responses. +Thanks to @cmrd-senya for finding and reporting the issue. + # 0.5.1.1 Update rails to 4.2.2, rack to 1.6.2 and jquery-rails to 4.0.4. This fixes diff --git a/app/presenters/person_presenter.rb b/app/presenters/person_presenter.rb index 37f7593534ba9760888720a81accce0aef783b5c..f53087eee0d87c1f7df4ff76bd2b69497314ab0c 100644 --- a/app/presenters/person_presenter.rb +++ b/app/presenters/person_presenter.rb @@ -1,38 +1,47 @@ class PersonPresenter < BasePresenter def base_hash - { id: id, - guid: guid, - name: name, + { + id: id, + guid: guid, + name: name, diaspora_id: diaspora_handle } end def full_hash - base_hash.merge({ - relationship: relationship, - block: is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false, - contact: (!own_profile? && has_contact?) ? { id: current_user_person_contact.id } : false, + base_hash.merge( + relationship: relationship, + block: is_blocked? ? BlockPresenter.new(current_user_person_block).base_hash : false, + contact: (!own_profile? && has_contact?) ? {id: current_user_person_contact.id} : false, is_own_profile: own_profile? - }) + ) end def full_hash_with_avatar - full_hash.merge({avatar: AvatarPresenter.new(profile).base_hash}) + full_hash.merge(avatar: AvatarPresenter.new(profile).base_hash) end def full_hash_with_profile - full_hash.merge({profile: ProfilePresenter.new(profile).full_hash}) + attrs = full_hash + + if own_profile? || person_is_following_current_user + attrs.merge!(profile: ProfilePresenter.new(profile).private_hash) + else + attrs.merge!(profile: ProfilePresenter.new(profile).public_hash) + end + + attrs end - def as_json(options={}) + def as_json(_options={}) attrs = full_hash_with_avatar if own_profile? || person_is_following_current_user - attrs.merge!({ - :location => @presentable.location, - :birthday => @presentable.formatted_birthday, - :bio => @presentable.bio - }) + attrs.merge!( + location: @presentable.location, + birthday: @presentable.formatted_birthday, + bio: @presentable.bio + ) end attrs @@ -51,7 +60,7 @@ class PersonPresenter < BasePresenter contact = current_user_person_contact return :not_sharing unless contact - [:mutual, :sharing, :receiving].find do |status| + %i(mutual sharing receiving).find do |status| contact.public_send("#{status}?") end || :not_sharing end diff --git a/app/presenters/profile_presenter.rb b/app/presenters/profile_presenter.rb index 3581ef9f925a357a7f961c015857a580a0f6e15d..71e624fcbc667bc443005d74a58be795475fbd82 100644 --- a/app/presenters/profile_presenter.rb +++ b/app/presenters/profile_presenter.rb @@ -2,20 +2,26 @@ class ProfilePresenter < BasePresenter include PeopleHelper def base_hash - { id: id, - tags: tags.pluck(:name), - bio: bio_message.plain_text_for_json, - location: location_message.plain_text_for_json, - gender: gender, - birthday: formatted_birthday, - searchable: searchable + { + id: id, + searchable: searchable } end - def full_hash - base_hash.merge({ + def public_hash + base_hash.merge( avatar: AvatarPresenter.new(@presentable).base_hash, - }) + tags: tags.pluck(:name) + ) + end + + def private_hash + public_hash.merge( + bio: bio_message.plain_text_for_json, + birthday: formatted_birthday, + gender: gender, + location: location_message.plain_text_for_json + ) end def formatted_birthday diff --git a/spec/presenters/person_presenter_spec.rb b/spec/presenters/person_presenter_spec.rb index 97f01887cf3aef1505302b5a365db384965f9533..54424e2ee6533ce1e400e9568c83e33c62dddf58 100644 --- a/spec/presenters/person_presenter_spec.rb +++ b/spec/presenters/person_presenter_spec.rb @@ -16,12 +16,12 @@ describe PersonPresenter do let(:presenter){ PersonPresenter.new(person, current_user) } it "doesn't share private information when the users aren't connected" do - expect(presenter.as_json).not_to have_key(:location) + expect(presenter.full_hash_with_profile[:profile]).not_to have_key(:location) end it "has private information when the person is sharing with the current user" do expect(person).to receive(:shares_with).with(current_user).and_return(true) - expect(presenter.as_json).to have_key(:location) + expect(presenter.full_hash_with_profile[:profile]).to have_key(:location) end it "returns the user's private information if a user is logged in as herself" do