Risk level validation

[werejago]

Hello DOS Community,

I have a simple request if anyone can assist,

I need your expert opinion if this code is safe or if you see any potential flaws the way this script is written I'm open to discuss,

Please refer to viewtopic.php?f=3&t=9794 if you're confused on why and how this script was designed.

kind regards, thank you!

Code: Select all

@echo off
Color F
rem /*_________________________________________Deletes all Files in folder besides itself or other folders */ 
9>>"%~f0" (>nul 2>&1 del /f /q *.*)

rem /*_________________________________________Copies and rename designated files */ 
xcopy "C:\Users\wkirk\Desktop\Sample\Laser Engraver Master Files" "C:\Users\wkirk\Desktop\Sample\Barcode Serialization Files" 
rename "Sensor Engraving Master File.EZD" "Sensor Engraving.EZD"
rename "Sensor Serialization Master File.txt" "Sensor Serialization.txt"

cls

@echo off & CD "%~dp0" & Goto :Main
rem /*_________________________________________Functions; sets parameters, sets input, verfies input to parameters */
:GetIN [ReturnVar] [Length] [Allowed Characters] [Format]
Setlocal EnableDelayedExpansion
Set "Input=(Set "nVar="&Echo/Enter %2 Digit %1 %4:&(For /L %%. in (1 1 %2)Do For /F "Delims=" %%G in ('Choice /N /C %3')Do (<Nul Set /P"=%%G"&Set "nvar=^^!nVar^^!%%G"))&Echo/&Echo/Confirm: ^^!nVar^^! Y/N & For /F "Delims=" %%v in ('Choice /N')Do (If /I "%%~v"=="n" (Goto :retry)))"
:retry
%Input% & Echo/%1 [!nVar!] Confirmed
Endlocal & Set "%1=%nVar%" 2> Nul
Exit /B 0
:UpdateLOT
 <"!file!" set /p "line1="
 >"!file!.~tmp" Echo/!line1!
 >>"!file!.~tmp" echo/!#LOT!
 <"!file!" >>"!file!.~tmp" more +2
 move /y "!file!.~tmp" "!file!"
Exit /B
:UpdateID
 <"!File!" (
  Set /P "Line[1]="
  Set /P "Line[2]="
  Set /P "Line[3]="
 )
 >"!File!.~tmp" (
  Echo/!Line[1]!
  Echo/!Line[2]!
  Echo/!Line[3]!
  Echo/!#ID!
 )
 rem /* the below acts upon entries after the entered #ID */
Set "Start=F"
(For /F "UsebackQ Skip=3 Delims=" %%G in ("!File!")Do If not "%%~G" == "!#ID!" (If "!Start!" == "T" (Echo/%%~G))Else Set "Start=T") >>"%File%.~tmp"
move /y "!file!.~tmp" "!file!"
Exit /B
rem /*_________________________________________Script Body; reads/writes to Serialization.txt line 2 */ 
:Main

Set "Menu=Echo/[R]epeat [C]ontinue [E]xit&For /F "Delims=" %%G in ('Choice /N /C:RCE')Do If "%%G"=="R" ( Goto :lbl )Else If "%%G"=="E" (start notepad "Sensor Serialization.txt" && start C:\Users\ptech\Documents\Ezcad2.14.9^(20170509^)\EzCad2.exe "Sensor Engraving.EZD" & Endlocal & Exit /B 0)"
setlocal EnableDelayedExpansion
:LOTNumber
set "file=Sensor Serialization.txt"
Call :GetIn #LOT 8 0123456789
rem /* Call function to update Lot number # in file */
Call :UpdateLOT
ECHO ______________________________________________
ECHO [R]	=	[Retry your entry]
ECHO [C]	=	[ONLY IF FREEZE - Set starting sensor ID]
ECHO [E]	=	[Finish and Exit]
ECHO ______________________________________________
Echo off

%menu:lbl=LOTNumber%
:SensorID
ECHO ______________________________________________
ECHO [01-26]	=	[Carrier #]
ECHO [A-H]	=	[Sensor ID letter]
ECHO [01-16]	=	[Sensor ID #]
ECHO ______________________________________________
Echo off
Call :GetIn #ID 5 0123456789ABCDEFGH {[01-26][A-H][01-16]}
TYPE "%File%" | findstr.exe /BLIC:"%#ID%" > nul 2> nul || (Echo/Invalid Sensor ID # & Goto :SensorID) 
Call :UpdateID
%menu:lbl=SensorID%

(start notepad "Sensor Serialization.txt" && start C:\Users\ptech\Documents\Ezcad2.14.9^(20170509^)\EzCad2.exe "Sensor Engraving.EZD" & Endlocal & Exit /B 0)


Endlocal

[werejago]

Is using ECHO off unnecessary and should I leave it blank or replace with a pause?

Code: Select all

ECHO ______________________________________________
ECHO [R]	=	[Retry your entry]
ECHO [C]	=	[ONLY IF FREEZE - Set starting sensor ID]
ECHO [E]	=	[Finish and Exit]
ECHO ______________________________________________
Echo off

[werejago]

I removed "& Endlocal & Exit /B 0" from the code below because I already have an Endlocal at the end of the script.

Code: Select all

(start notepad "Sensor Serialization.txt" && start C:\Users\ptech\Documents\Ezcad2.14.9^(20170509^)\EzCad2.exe "Sensor Engraving.EZD" & Endlocal & Exit /B 0)

and just have (start notepad "Sensor Serialization.txt" && start C:\Users\ptech\Documents\Ezcad2.14.9^(20170509^)\EzCad2.exe "Sensor Engraving.EZD") instead.

Code: Select all

(start notepad "Sensor Serialization.txt" && start C:\Users\ptech\Documents\Ezcad2.14.9^(20170509^)\EzCad2.exe "Sensor Engraving.EZD") 

Is this more effective?

[penpen]

Ad OP:
Depending how you started your batch and if its name or its path contains spaces, the variable %~f0 might not neccessarily be correct.
The only solution for that (special case) is to first call a function(-label) and use %~f0 from within that function.

The way you delete all files also does mess up your device mappings; you should avoid using two different redirections at once and instead use parentheses to seperate them.

Also you might first want to ensure that your current directory is the correct directory you want to work in (you could execute batch files from different folders).

Renaming a file should fail for the reason that you have deleted all files and folders in the current folder.

Executing external commands such as "choice", "move", etc. typically is sufficient, but in case you thought about an attack scenario, the you should use the full file name to that external command.


Ad post 2)
The line "ECHO off" indeed seems to be unneccessary, but there is a possibility to start at a specific label from another batch file.
If your batch is designed to do that, then the above command might be expected behaviour.
If your batch is not designed to do that, then you might want to protect your batch file from beeing accessed from the outside in that way, by preceeding all used labels by the same label - but you might need to redesign some of your code in such a case.
Beside that there are alaways ways to execute parts of your batch file, which means you can't protect it anyway from beeing processed from the outside, so you might decide against such a protection - in which case you don't need to change your code because of that.