added audit beacon to detect misuse of this bidder. Detects auctions… #1134

jstocker76 · 2017-04-18T17:32:53Z

… within iframes or popup windows.

Type of change

Description of change

Added an audit beacon to detect unintended use of this bidder.

test parameters for validating bids

{
  bidder: 'rhythmone',
  params: {
                        placementId: "34887",
						zone: "test",
						path: "mvo",
						debug: true,
						method:"get"
                    }
}

contact email of the adapter’s maintainer
official adapter submission

Other information

…within iframes or popup windows.

…uction ended flag was set, preventing additional auctions from completing. This has been removed along with some google analytics tracking code.

…cert.

hipstern · 2017-04-26T17:03:35Z

looks good

ehoch · 2017-04-27T21:44:17Z

@mkendall07 @protonate Sorry to be a pain, but is this something we can get reviewed ASAP? This is actually a really important bug fix for multiple auctions.

protonate

This looks good, some style comments. We are looking into an audit mechanism for bidders to generally obtain information about their bids in the auction, so this code may refactor at a later point.

protonate · 2017-04-28T16:33:32Z

src/adapters/rhythmone.js

+      i = new Image();
+
+    if(ao && ao.length > 0)
+      data.ancestor_origins = ao[ao.length-1];


code style: please add braces to enclose function body.

protonate · 2017-04-28T16:35:01Z

src/adapters/rhythmone.js

+      data.ancestor_origins = ao[ao.length-1];
+
+    data.popped = (window.opener!==null?1:0);
+    data.framed = (window.top===window?0:1);


code style: parentheses can be removed.

protonate · 2017-04-28T16:36:11Z

src/adapters/rhythmone.js

+    data.response_ms = (new Date()).getTime() - loadStart;
+    data.placement_codes = configuredPlacements.join(",");
+    data.bidder_version = version;
+    data.prebid_timeout = (prebid_instance.cbTimeout || prebid_instance.bidderTimeout);


code style: parentheses can be removed.

protonate · 2017-04-28T16:36:50Z

src/adapters/rhythmone.js

+    data.prebid_timeout = (prebid_instance.cbTimeout || prebid_instance.bidderTimeout);
+
+    for(var k in data)
+      q.push(encodeURIComponent(k)+"="+encodeURIComponent((typeof data[k] === "object" ? JSON.stringify(data[k]) : data[k])));


code style: please add braces to enclose function body.

jstocker76 · 2017-04-28T23:37:26Z

updated code with style suggestions.

protonate

LGTM, one question about ancestorOrigins.

protonate · 2017-05-02T16:11:05Z

src/adapters/rhythmone.js

+        doc_type : "Prebid Audit",
+        placement_id: placementId
+      },
+      ao = document.location.ancestorOrigins,


Is browser support for ancestorOrigins a concern here?

It's not a concern, it is only available in webkit browsers TMK so it will only be used if there.

matthewlane · 2017-05-02T16:59:37Z

src/adapters/rhythmone.js

+	}
+
+    data.popped = window.opener!==null?1:0;
+    data.framed = window.top===window?0:1;


Please place this window.top reference in a try/catch as well

With all due respect, this is unnecessary. A security exception is only thrown when accessing the properties of top. The window object itself can be compared safely.

@jstocker76 run this: https://jsfiddle.net/04o2funn/

odd, I get no exception when I run that code in a sample file with a cross domain iframe. Furthermore, I don't get that exception when running the same code by targeting chrome's console to that iframe. What sorcery is this?

I see. It's the alert causing the exception. If you just run "window.top" no exception.

yeah that makes sense. This is good. Prebid core should probably just utility this function because tons of bidders do this check.

https://jsfiddle.net/10j1yw82/

alert must automatically run .toString() on window.top, which would trigger the exception

Reeling in the proliferation of duplicate functionality must be a full time job. The list of bidders is getting quite long

…built * 'master' of https://github.com/prebid/Prebid.js: (21 commits) add lodash as dependency (prebid#1174) fix size mapping for s2s (prebid#1175) Improve footer styling (prebid#1171) Bugfix: internal bids requested overwritten (prebid#1173) pre-release version bump Prebid 0.23.0 Release Yieldbot adapter - multiple requestBids per pageview (prebid#1146) Widespace adapter validate size fix (prebid#1140) Audience Network: bid when at least one valid slot size (prebid#1148) Quantcast adaptor (prebid#1063) AOL Adapter - ONE Mobile endpoint implemented. (prebid#1115) Prebid Server to Server (prebid#1165) Pubgears Header Bidding Adapter (prebid#953) remove old adloader#trackPixel (prebid#1159) added audit beacon to detect misuse of this bidder. Detects auctions… (prebid#1134) Bidfluence CDN endpoint URL update (prebid#1163) AdSupply adapter (prebid#1162) Sonobi Adapter - Enable size overrides (prebid#1141) Added an editorconfig file to match jshint and jssrc files. (prebid#1147) force cpm to be a number (prebid#1161) ...

…prebid#1134) * added audit beacon to detect misuse of this bidder. Detects auctions within iframes or popup windows. * fix for bug prebid#1123 - Rhythmone: Multiple requestBids Not Working. An auction ended flag was set, preventing additional auctions from completing. This has been removed along with some google analytics tracking code. * Altered the beacon logging domain to something with a functional SSL cert. * updating s3 bucket name * updating s3 bucket name _again_. * Adding optional curly braces. Removing optional parentheses.

jstocker76 added 4 commits April 18, 2017 10:27

added audit beacon to detect misuse of this bidder. Detects auctions …

7a3889e

…within iframes or popup windows.

fix for bug #1123 - Rhythmone: Multiple requestBids Not Working. An a…

e957dec

…uction ended flag was set, preventing additional auctions from completing. This has been removed along with some google analytics tracking code.

Altered the beacon logging domain to something with a functional SSL …

7090a6d

…cert.

updating s3 bucket name

1c3ae2d

updating s3 bucket name _again_.

fbabd87

protonate self-requested a review April 27, 2017 22:11

protonate self-assigned this Apr 28, 2017

protonate suggested changes Apr 28, 2017

View reviewed changes

Adding optional curly braces. Removing optional parentheses.

7e55ad3

protonate approved these changes May 2, 2017

View reviewed changes

matthewlane self-requested a review May 2, 2017 16:16

matthewlane requested changes May 2, 2017

View reviewed changes

protonate added this to the Prebid 0.23.0 milestone May 2, 2017

protonate added needs update LGTM and removed needs update labels May 2, 2017

matthewlane merged commit 2652fc0 into prebid:master May 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added audit beacon to detect misuse of this bidder. Detects auctions… #1134

added audit beacon to detect misuse of this bidder. Detects auctions… #1134

jstocker76 commented Apr 18, 2017

hipstern commented Apr 26, 2017

ehoch commented Apr 27, 2017

protonate left a comment

protonate Apr 28, 2017

protonate Apr 28, 2017

protonate Apr 28, 2017

protonate Apr 28, 2017

jstocker76 commented Apr 28, 2017

protonate left a comment

protonate May 2, 2017

hipstern May 2, 2017

matthewlane May 2, 2017

jstocker76 May 2, 2017

mkendall07 May 2, 2017

jstocker76 May 2, 2017

jstocker76 May 2, 2017

mkendall07 May 2, 2017

jstocker76 May 2, 2017 •

edited

Loading

jstocker76 May 2, 2017

added audit beacon to detect misuse of this bidder. Detects auctions… #1134

added audit beacon to detect misuse of this bidder. Detects auctions… #1134

Conversation

jstocker76 commented Apr 18, 2017

Type of change

Description of change

Other information

hipstern commented Apr 26, 2017

ehoch commented Apr 27, 2017

protonate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jstocker76 commented Apr 28, 2017

protonate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jstocker76 May 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jstocker76 May 2, 2017 •

edited

Loading