Tested Ack,
Mathi.
----- [email protected] wrote:
> osaf/services/saf/logsv/lgs/lgs_evt.c | 7 ++++
> osaf/services/saf/logsv/lgs/lgs_imm.c | 11 +++++-
> osaf/services/saf/logsv/lgs/lgs_util.c | 57
> ++++++++++++++++++++++++++++++++++
> osaf/services/saf/logsv/lgs/lgs_util.h | 1 +
> tests/logsv/tet_LogOiOps.c | 40 +++++++++++++++++++++++
> tests/logsv/tet_saLogStreamOpen_2.c | 51
> ++++++++++++++++++++++++++++++
> 6 files changed, 166 insertions(+), 1 deletions(-)
>
>
> logsv did not check if the created/modified the attribute value
> was a valid file name or not.
>
> To fix this, add validation - guarantee that forward slash '/'
> is not in the file name:
>
> diff --git a/osaf/services/saf/logsv/lgs/lgs_evt.c
> b/osaf/services/saf/logsv/lgs/lgs_evt.c
> --- a/osaf/services/saf/logsv/lgs/lgs_evt.c
> +++ b/osaf/services/saf/logsv/lgs/lgs_evt.c
> @@ -820,6 +820,13 @@ static SaAisErrorT create_new_app_stream
> goto done;
> }
>
> + /* Verify if there is any special character in logFileName */
> + if (lgs_has_special_char(open_sync_param->logFileName) == true) {
> + TRACE("Invalid logFileName - %s", open_sync_param->logFileName);
> + rc = SA_AIS_ERR_INVALID_PARAM;
> + goto done;
> + }
> +
> /* Verify that path and file are unique */
> stream = log_stream_getnext_by_name(NULL);
> while (stream != NULL) {
> diff --git a/osaf/services/saf/logsv/lgs/lgs_imm.c
> b/osaf/services/saf/logsv/lgs/lgs_imm.c
> --- a/osaf/services/saf/logsv/lgs/lgs_imm.c
> +++ b/osaf/services/saf/logsv/lgs/lgs_imm.c
> @@ -135,7 +135,7 @@ static void report_om_error(SaImmOiHandl
> ao_err_params);
> }
>
> -/**
> +/**
> * Pack and send a log service config checkpoint using mbcsv
> * @param stream
> *
> @@ -1471,6 +1471,15 @@ static SaAisErrorT check_attr_validity(S
> TRACE("saLogStreamFileName is so long (max:
> %d)", LOG_NAME_MAX);
> goto done;
> }
> + /* Not allow special characters exising in file name */
> + if (lgs_has_special_char(i_fileName) == true) {
> + /* Report failed if has special character in
> file name */
> + rc = SA_AIS_ERR_BAD_OPERATION;
> + report_oi_error(immOiHandle, opdata->ccbId,
> + "Invalid
> saLogStreamFileName value");
> + TRACE("\t448 Invalid saLogStreamFileName
> value");
> + goto done;
> + }
>
> if (chk_filepath_stream_exist(i_fileName, i_pathName,
> stream, opdata->operationType)) {
> diff --git a/osaf/services/saf/logsv/lgs/lgs_util.c
> b/osaf/services/saf/logsv/lgs/lgs_util.c
> --- a/osaf/services/saf/logsv/lgs/lgs_util.c
> +++ b/osaf/services/saf/logsv/lgs/lgs_util.c
> @@ -669,3 +669,60 @@ done:
> TRACE_LEAVE2("rc = %d",rc);
> return rc;
> }
> +
> +/**
> + * Validate if string contains special characters or not
> + *
> + * @param: str [in] input string for checking
> + * @return: true if there is special character, otherwise false.
> + */
> +bool lgs_has_special_char(const char *str)
> +{
> + int index = 0;
> + char c;
> +
> + while ((c = str[index++]) != '\0') {
> + switch (c) {
> + /* Unix-like system does not support slash in filename */
> + case '/':
> + /**
> + * Unix-like system supports these characters in filename
> + * but it could not support in other platforms.
> + *
> + * If mounting the root log dir to other platforms (e.g:
> Windows)
> + * These characters could consider to add in as restriction.
> + *
> + * But for now, to avoid NBC, these ones are allowed.
> + *
> + */
> + /* case '|': */
> + /* case ';': */
> + /* case ',': */
> + /* case '!': */
> + /* case '@': */
> + /* case '#': */
> + /* case '$': */
> + /* case '(': */
> + /* case ')': */
> + /* case '<': */
> + /* case '>': */
> + /* case '\\': */
> + /* case '"': */
> + /* case '\'': */
> + /* case '`': */
> + /* case '~': */
> + /* case '{': */
> + /* case '}': */
> + /* case '[': */
> + /* case ']': */
> + /* case '+': */
> + /* case '&': */
> + /* case '^': */
> + /* case '?': */
> + /* case '*': */
> + /* case '%': */
> + return true;
> + }
> + }
> + return false;
> +}
> diff --git a/osaf/services/saf/logsv/lgs/lgs_util.h
> b/osaf/services/saf/logsv/lgs/lgs_util.h
> --- a/osaf/services/saf/logsv/lgs/lgs_util.h
> +++ b/osaf/services/saf/logsv/lgs/lgs_util.h
> @@ -67,4 +67,5 @@ extern int lgs_make_reldir_h(const char*
> extern int lgs_check_path_exists_h(const char *path_to_check);
> extern gid_t lgs_get_data_gid(char *groupname);
> extern int lgs_own_log_files_h(log_stream_t *stream, const char
> *groupname);
> +extern bool lgs_has_special_char(const char *str);
> #endif /* ifndef __LGS_UTIL_H */
> diff --git a/tests/logsv/tet_LogOiOps.c b/tests/logsv/tet_LogOiOps.c
> --- a/tests/logsv/tet_LogOiOps.c
> +++ b/tests/logsv/tet_LogOiOps.c
> @@ -3362,6 +3362,45 @@ void verFilenameLength_02(void)
> rc_validate(WEXITSTATUS(rc), 1);
> }
>
> +/**
> + * Add test case for ticket #1421
> + * logsv not verify if created/modified log file name is valid.
> + * Test steps:
> + * 1. Get current saLogStreamFileName
> + * 2. Iterate all not-allowed characters
> + * 3. Update saLogStreamFileName containing one of that character
> + * 4. Verify that it fails to perform that command.
> + * 5. Restore to previous value.
> + */
> +void verLogFileName(void)
> +{
> + int rc;
> + char command[256];
> + /* const char *str = "|;,!@#$()<>/\\\"'`~{}[]+&^?*%/"; */
> +
> + /* int i = 0; */
> + /* for (; i < strlen(str); i++) { */
> + /* if (str[i] != '\'') { */
> + /* sprintf(command, "immcfg -a
> saLogStreamFileName='tmp_%c' %s 2>
> /dev/null", */
> + /* str[i], alarmStreamName.value); */
> + /* } else { */
> + /* sprintf(command, "immcfg -a
> saLogStreamFileName=\"tmp_'\" %s 2>
> /dev/null", */
> + /* alarmStreamName.value); */
> + /* } */
> + /* rc = system(command); */
> + /* if (WEXITSTATUS(rc) == 0) { */
> + /* fprintf(stderr, "Failed - command = %s\n", command); */
> + /* break; */
> + /* } */
> + /* } */
> +
> + /* Invalid filename with forward slash in */
> + sprintf(command, "immcfg -a saLogStreamFileName='invalidFile/Name'
> %s 2> /dev/null",
> + alarmStreamName.value);
> + rc = system(command);
> +
> + rc_validate(WEXITSTATUS(rc), 1);
> +}
> __attribute__ ((constructor)) static void
> saOiOperations_constructor(void)
> {
> /* Stream objects */
> @@ -3447,6 +3486,7 @@ void verFilenameLength_02(void)
> test_case_add(5, saLogOi_61, "CCB Object Modify,
> logStreamAppHighLimit = logStreamAppLowLimit = 0. OK");
> test_case_add(5, saLogOi_62, "CCB Object Modify,
> logMaxApplicationStreams. Not allowed");
> test_case_add(5, saLogOi_64, "CCB Object Modify,
> logFileSysConfig. Not allowed");
> + test_case_add(5, verLogFileName, "CCB Object Modify,
> saLogStreamFileName with special character. ER");
>
> /* Add test cases to test #1288 */
> test_case_add(5, verLogFileIoTimeout, "CCB Object Modify:
> logFileIoTimeout is in range [500 - 5000], OK");
> diff --git a/tests/logsv/tet_saLogStreamOpen_2.c
> b/tests/logsv/tet_saLogStreamOpen_2.c
> --- a/tests/logsv/tet_saLogStreamOpen_2.c
> +++ b/tests/logsv/tet_saLogStreamOpen_2.c
> @@ -583,6 +583,56 @@ done:
> }
> }
>
> +
> +/**
> + * Added test case to verify ticket #1421
> + * Verify logsv failed to create app stream with invalid file name
> + */
> +void saLogStreamOpen_2_49(void)
> +{
> + SaAisErrorT rc = SA_AIS_OK;
> + SaNameT logStreamName = {.length = 0 };
> +
> + strcpy((char *)logStreamName.value, SA_LOG_STREAM_APPLICATION1);
> + logStreamName.length = strlen((char *)logStreamName.value);
> +
> + SaLogFileCreateAttributesT_2 appLogFileCreateAttributes;
> +
> + /* App stream information */
> + appLogFileCreateAttributes.logFilePathName = "appstream";
> + appLogFileCreateAttributes.logFileName = "appstream/";
> + appLogFileCreateAttributes.maxLogFileSize =
> DEFAULT_APP_LOG_FILE_SIZE;
> + appLogFileCreateAttributes.maxLogRecordSize =
> DEFAULT_APP_LOG_REC_SIZE;
> + appLogFileCreateAttributes.haProperty = SA_TRUE;
> + appLogFileCreateAttributes.logFileFullAction =
> SA_LOG_FILE_FULL_ACTION_ROTATE;
> + appLogFileCreateAttributes.maxFilesRotated = 4;
> + appLogFileCreateAttributes.logFileFmt = NULL;
> +
> + rc = saLogInitialize(&logHandle, &logCallbacks, &logVersion);
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed at saLogInitialize: %d \n",
> (int)rc);
> + test_validate(rc, SA_AIS_OK);
> + return;
> + }
> +
> + rc = saLogSelectionObjectGet(logHandle, &selectionObject);
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed at saLogSelectionObjectGet: %d \n",
> (int)rc);
> + test_validate(rc, SA_AIS_OK);
> + goto done;
> + }
> +
> + rc = saLogStreamOpen_2(logHandle, &logStreamName,
> &appLogFileCreateAttributes,
> + SA_LOG_STREAM_CREATE, SA_TIME_ONE_SECOND,
> &logStreamHandle);
> + test_validate(rc, SA_AIS_ERR_INVALID_PARAM);
> +
> +done:
> + rc = saLogFinalize(logHandle);
> + if (rc != SA_AIS_OK) {
> + fprintf(stderr, "Failed to call salogFinalize: %d \n", (int)
> rc);
> + }
> +}
> +
> extern void saLogStreamOpenAsync_2_01(void);
> extern void saLogStreamOpenCallbackT_01(void);
> extern void saLogWriteLog_01(void);
> @@ -660,6 +710,7 @@ extern void saLogStreamClose_01(void);
> test_case_add(2, saLogStreamOpen_2_46, "saLogStreamOpen_2 with
> maxFilesRotated = 0, ERR");
> test_case_add(2, saLogStreamOpen_2_47, "saLogStreamOpen_2 with
> maxFilesRotated = 128, ERR");
> test_case_add(2, saLogStreamOpen_2_48, "saLogStreamOpen_2 with
> logFileName > 218 characters, ERR");
> + test_case_add(2, saLogStreamOpen_2_49, "saLogStreamOpen_2 with
> invalid filename");
>
> test_case_add(2, verFixLogRec_Max_Err, "saLogStreamOpen_2 with
> maxLogRecordSize > MAX_RECSIZE, ERR");
> test_case_add(2, verFixLogRec_Min_Err, "saLogStreamOpen_2 with
> maxLogRecordSize < 150, ERR");
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
