There is an access bypass issue. I'm not reporting this to the security team since the module does not meet the Security-advisory policy yet (not a full release).

This issue was discovered while I was reproducing #2383903: Cannot save queues with entities user doesn't have access to

Steps to reproduce

  1. Create an Entityqueue of nodes
  2. Add a published node to the queue
  3. Edit the node, and unpublished it
  4. Login as a user who does not have access to view unpublished nodes
  5. Edit the queue and see the node title

Comments

jojonaloha’s picture

Priority: Normal » Critical
StatusFileSize
new1.17 KB

Attached is a patch to fix the access issue only. It does not resolve #2383903: Cannot save queues with entities user doesn't have access to

amateescu’s picture

StatusFileSize
new1.1 KB

We should probably follow what entityreference is doing and rely on the getLabel() method of the selection handlers. Something like this.

nerdacus’s picture

Both patches work in the sense that they remove the label from the immediate view of the list. Patch #2 falls short in that the label is still converted to a link, which, if URL aliases to entities are being generated from the entity labels, the parts (or all) of the label can be viewed by a nosey user. Also, the entity ID is still appended to the "- Restricted access -" label, giving a malicious user more information than it should.

In short, I'd go with patch #1.

amateescu’s picture

Status: Active » Reviewed & tested by the community

Also, the entity ID is still appended to the "- Restricted access -" label, giving a malicious user more information than it should.

This is also what Entity reference is doing and it was deemed as not a security problem.

And you're right, my patch doesn't take into account the url part (why do we even link to the entities in first place?), so the patch in #1 looks good to me.

  • jojonaloha committed e817f10 on 7.x-1.x
    Issue #2442875 by jojonaloha, amateescu: Displays labels of entities...
jojonaloha’s picture

Status: Reviewed & tested by the community » Fixed

Committed e817f10

amateescu’s picture

I think it's about time to finally push a stable release, or at least an rc or something. What do you think?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.