Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ntp-si): read asset list explicitly #1046

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,4 @@ brave-lists
leo-local-models
# localstack
.localstack/
.vscode/
153 changes: 39 additions & 114 deletions lib/ntpUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,70 +6,7 @@ import childProcess from 'child_process'
import fs from 'fs'
import path from 'path'
import util from '../lib/util.js'
import { Readable } from 'stream'
import { finished } from 'stream/promises'

const jsonSchemaVersion = 1

/**
* @typedef {{ imageUrl: string }} Logo
* @typedef {{ logo?: Logo, imageUrl: string}} Wallpaper
* @typedef {{ logo: Logo, wallpapers: Wallpaper[]}} Campaign
* @typedef {Campaign & { campaigns?: Campaign[], campaigns2?: Campaign[], topSites?: { iconUrl }[] }} NTPAssetSchema
*/

const createPhotoJsonFile = (path, body) => {
fs.writeFileSync(path, body)
}

/**
*
* @param {Campaign} campaign
* @returns {string[]}
*/
function getImageFileNameListFromCampaign (campaign) {
const fileList = new Set()
if (campaign.logo) {
fileList.add(campaign.logo.imageUrl)
}
if (campaign.wallpapers) {
campaign.wallpapers.forEach((wallpaper) => {
fileList.add(wallpaper.imageUrl)
// V2 feature - support per-wallpaper logo
if (wallpaper.logo && wallpaper.logo.imageUrl) {
fileList.add(wallpaper.logo.imageUrl)
}
})
}
return Array.from(fileList.values())
}

/**
*
* @param {NTPAssetSchema} photoJsonObj
* @returns {string[]}
*/
const getImageFileNameListFrom = (photoJsonObj) => {
const fileList = new Set(
getImageFileNameListFromCampaign(photoJsonObj)
)
if (photoJsonObj.campaigns) {
for (const campaign of photoJsonObj.campaigns) {
getImageFileNameListFromCampaign(campaign).forEach(s => fileList.add(s))
}
}
if (photoJsonObj.campaigns2) {
for (const campaign of photoJsonObj.campaigns2) {
getImageFileNameListFromCampaign(campaign).forEach(s => fileList.add(s))
}
}
if (photoJsonObj.topSites) {
photoJsonObj.topSites.forEach((topSiteObj) => {
fileList.add(topSiteObj.iconUrl)
})
}
return Array.from(fileList.values())
}
import { pipeline } from 'stream/promises'

const generatePublicKeyAndID = (privateKeyFile) => {
childProcess.execSync(`openssl rsa -in ${privateKeyFile} -pubout -out public.pub`)
Expand All @@ -94,61 +31,49 @@ const generatePublicKeyAndID = (privateKeyFile) => {
}
}

const isValidSchemaVersion = (version) => {
return version === jsonSchemaVersion
const downloadFile = async (sourceUrl, dest) => {
const response = await fetch(sourceUrl)

if (!response.ok) {
throw new Error(`download of ${sourceUrl} failed with code ${response.status}`)
}

if (!response.body) {
throw new Error(`download of ${sourceUrl} failed with empty body`)
}

const file = fs.createWriteStream(dest)
await pipeline(response.body, file)
}

const prepareAssets = (jsonFileUrl, targetResourceDir, targetJsonFileName) => {
return new Promise(function (resolve, reject) {
let jsonFileBody = '{}'
const prepareAssets = async (jsonFileUrl, targetResourceDir) => {
const response = await fetch(jsonFileUrl)

// Download and parse jsonFileUrl.
// If it doesn't exist, create with empty object.
fetch(jsonFileUrl).then(async function (response) {
if (response.status === 200) {
jsonFileBody = await response.text()
}
let photoData = {}
try {
console.log(`Start - json file ${jsonFileUrl} parsing`)
photoData = JSON.parse(jsonFileBody)
} catch (err) {
console.error(`Invalid json file ${jsonFileUrl}`)
return reject(err)
}
console.log(`Done - json file ${jsonFileUrl} parsing`)
// Make sure the data has a schema version so that clients can opt to parse or not
let incomingSchemaVersion = photoData.schemaVersion
console.log(`Schema version: ${incomingSchemaVersion} from ${jsonFileUrl}`)
if (!incomingSchemaVersion) {
// Source has no schema version, assume and set current version.
// TODO(petemill): Don't allow this once the source is established to always
// have a schema version.
incomingSchemaVersion = jsonSchemaVersion
} else if (!isValidSchemaVersion(incomingSchemaVersion)) {
const error = `Error: Cannot parse JSON data at ${jsonFileUrl} since it has a schema version of ${incomingSchemaVersion} but we expected ${jsonSchemaVersion}! This region will not be updated.`
console.error(error)
return reject(error)
}
if (!response.ok) {
throw new Error(`download of ${jsonFileUrl} failed with code ${response.status}`)
}

createPhotoJsonFile(path.join(targetResourceDir, 'photo.json'), JSON.stringify(photoData))

// Download image files that specified in jsonFileUrl
const imageFileNameList = getImageFileNameListFrom(photoData)
const downloadOps = imageFileNameList.map(async (imageFileName) => {
const targetImageFilePath = path.join(targetResourceDir, imageFileName)
const targetImageFileUrl = new URL(imageFileName, jsonFileUrl).href
const response = await fetch(targetImageFileUrl)
const ws = fs.createWriteStream(targetImageFilePath)
return finished(Readable.fromWeb(response.body).pipe(ws))
.then(() => console.log(targetImageFileUrl))
})
await Promise.all(downloadOps)
resolve()
}).catch(error => {
throw new Error(`Error from ${jsonFileUrl}: ${error.cause}`)
})
console.log(` Reading ${jsonFileUrl}`)

// example file format:
// https://github.com/brave/ntp-si-assets/blob/966021c07cf1dcb58128c0b0487b8bd974f4eda8/resources/test-data/examples/assets.json
const json = await response.json()

const allDownloads = json.assets.map(async asset => {
const targetAssetFilePath = path.join(targetResourceDir, asset.path)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.

Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal


Cc @thypon @kdenhartog

const targetAssetFileUrl = new URL(asset.path, jsonFileUrl).href
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[semgrep] Are you using the URL(url, base) constructor as a security control to limit the origin with base jsonFileUrl? The base is ignored whenever url looks like an absolute URL, e.g. when it begins protocol://. \\\\ or //x.y. Verify that the URL's origin is as expected rather than relying on the URL constructor.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/services/url-constructor-base.yaml


Cc @thypon @kdenhartog


await downloadFile(targetAssetFileUrl, targetAssetFilePath)

const hash = util.generateSHA256HashOfFile(targetAssetFilePath)
if (hash !== asset.sha256) {
throw new Error(`${targetAssetFileUrl}: hash does not match, expected ${asset.sha256} got ${hash}`)
}

console.log(' ' + targetAssetFileUrl)
})

await Promise.all(allDownloads)
}

export default {
Expand Down
2 changes: 1 addition & 1 deletion scripts/ntp-sponsored-images/generate.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ async function generateNTPSponsoredImages (dataUrl, targetComponents) {
const targetResourceDir = path.join(rootResourceDir, regionPlatformName)
mkdirp.sync(targetResourceDir)
const regionPlatformPath = regionPlatformName.replace('-', '/')
const sourceJsonFileUrl = `${dataUrl}${regionPlatformPath}/photo.json`
const sourceJsonFileUrl = `${dataUrl}${regionPlatformPath}/assets.json`
await ntpUtil.prepareAssets(sourceJsonFileUrl, targetResourceDir)
}
}
Expand Down