bug: validate Teradata logon host/user before BTEQ envsubst - #85
Open
devin-ai-integration[bot] wants to merge 2 commits into
Open
devin-ai-integration[bot] wants to merge 2 commits into
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Pin TD_SERVER to an allow-list and reject TD_USERNAME values containing whitespace or metacharacters so environment-controlled values cannot redirect the LDAP logon or inject BTEQ commands. Restrict envsubst to the logon variables only.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-Authored-By: Achal Channarasappa <achal.channarasappa@cognition.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes code-scan finding sfind-8b2ffe3dc2ad4b3aa142487d03432771:
.LOGON ${TD_SERVER}/${TD_USERNAME},;in the BTEQ scripts (and the heredoc inrun_full_pipeline.sh) is expanded byenvsubstfrom unvalidated environment values, so a newline inTD_USERNAMEinjected extra BTEQ dot-commands (e.g..OS) and a rewrittenTD_SERVERredirected the LDAP logon to an attacker host.config/pipeline_config.cfg: newTD_ALLOWED_SERVERSallow-list andvalidate_td_logon()(host must match^[A-Za-z0-9][A-Za-z0-9.-]*$and be allow-listed; user must match^[A-Za-z0-9_][A-Za-z0-9_.-]{0,127}$;LOOKBACK_MONTHSmust be an integer 1–120). Runs when the config is sourced, so every orchestrator fails fast on a bad value.BTEQ_SUBST_VARS='${TD_SERVER} ${TD_USERNAME} ${LOOKBACK_MONTHS}'—envsubstis now given an explicit SHELL-FORMAT inrun_bteq_pipeline.shandrun_full_pipeline.sh, so no other environment variable can be substituted into BTEQ input.validate_td_logonis re-run immediately before eachenvsubst | bteq.LOOKBACK_MONTHSis the only non-logon placeholder in the BTEQ scripts (02_stg_txn_summary.bteq), so it is included in the allow-list and validated; it now honours an environment override (${LOOKBACK_MONTHS:-12}) likeTD_USERNAME, and the config-load validation runs after it is set.Verified with a stub
bteq: default values produce the unchanged.LOGON tdprod.corp.bankdemo.com/svc_etl_pipeline,;andADD_MONTHS(CURRENT_DATE, -12);TD_USERNAME=$'svc\n.OS touch /tmp/PWNED;',TD_SERVER=evil.attacker.net,LOOKBACK_MONTHS=$'12\n.OS id',0and121all abort with rc=1 before BTEQ is invoked; an unrelated${DB_CORE}placeholder is left unsubstituted.Devin-Org: engineering
Link to Devin session: https://app.devin.ai/sessions/c58669f7ffc44e7f8ff92a5c16a9c917
Open in Devin Desktop: https://app.devin.ai/desktop/session/c58669f7ffc44e7f8ff92a5c16a9c917?variant=devin
Requested by: @achalc